Rel9 5 stable #9
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
XLogRecPtr was compared with InvalidTransactionId instead of InvalidXLogRecPtr. As both are defined to the same value this doesn't cause any actual problems, but it's still wrong. Back: 9.4-master, bug was introduced in 9.4
Commit 2834855 added a not-very-carefully-thought-out isolation test to check a BRIN index bug fix. The test depended on the availability of the pageinspect contrib module, which meant it did not work in several common testing scenarios such as "make check-world". It's not clear whether we want a core test depending on a contrib module like that, but in any case, failing to deal with the possibility that the module isn't present in the installation-under-test is not acceptable. Remove that test pending some better solution.
Commit 85e5e22 turns out not to have taken care of all cases of the partially-evaluatable-PlaceHolderVar problem found by Andreas Seltenreich's fuzz testing. I had set it up to check for risky PHVs only in the event that we were making a star-schema-based exception to the param_source_rels join ordering heuristic. However, it turns out that the problem can occur even in joins that satisfy the param_source_rels heuristic, in which case allow_star_schema_join() isn't consulted. Refactor so that we check for risky PHVs whenever the proposed join has any remaining parameterization. Back- to 9.2, like the previous (except for the regression test case, which only works back to 9.3 because it uses LATERAL). Note that this discovery implies that problems of this sort could've occurred in 9.2 and up even before the star-schema ; though I've not tried to prove that experimentally.
Apparently some versions of gcc prefer __sparc_v7__ and __sparc_v8__. Per report from Waldemar Brodkorb.
…lege. pg_dump produced fairly silly GRANT/REVOKE commands when dumping types from pre-9.2 servers, and when dumping functions or procedural languages from pre-7.3 servers. Those server versions lack the typacl, proacl, and/or lanacl columns respectively, and pg_dump substituted default values that were in fact incorrect. We ended up revoking all the owner's own privileges for the object while granting all privileges to PUBLIC. Of course the owner would then have those privileges again via PUBLIC, so long as she did not try to revoke PUBLIC's privileges; which may explain the lack of field reports. Nonetheless this is pretty silly behavior. The stakes were raised by my recent to make pg_dump dump shell types, because 9.2 and up pg_dump would proceed to emit bogus GRANT/REVOKE commands for a shell type if dumping from a pre-9.2 server; and the server will not accept GRANT/REVOKE commands for a shell type. (Perhaps it should, but that's a topic for another day.) So the resulting dump script wouldn't load without errors. The right thing to do is to act as though these objects have default privileges (null ACL entries), which causes pg_dump to print no GRANT/REVOKE commands at all for them. That fixes the silly results and also dodges the problem with shell types. In passing, modify getProcLangs() to be less creatively different about how to handle missing columns when dumping from older server versions. Every other data-acquisition function in pg_dump does that by substituting appropriate default values in the version-specific SQL commands, and I see no reason why this one should march to its own drummer. Its use of "SELECT *" was likewise not conformant with anyplace else, not to mention it's not considered good SQL style for production queries. Back- to all supported versions. Although 9.0 and 9.1 pg_dump don't have the issue with typacl, they are more likely than newer versions to be used to dump from ancient servers, so we ought to fix the proacl/lanacl issues all the way back.
Fix a bunch of typos, and remove two superflous includes. Author: Gurjeet Singh Discussion: CABwTF4Wh_dBCzTU=49pFXR6coR4NW1ynb+vBqT+Po=7fuq5iCw@mail.gmail.com Back: 9.4
newnfa() failed to set the regex error state when malloc() fails. Several places in regcomp.c failed to check for an error after calling subre(). Each of these mistakes could lead to null-pointer-dereference crashes in memory-starved backends. Report and by Andreas Seltenreich. Back- to all branches.
In 4b4b680 I passed a buffer index number (starting from 0) instead of a proper Buffer id (which start from 1 for shared buffers) in two places. This wasn't noticed so far as one of those locations isn't compiled at all (PrintPinnedBufs) and the other one (InvalidBuffer) requires a unlikely, but possible, set of circumstances to trigger a symptom. To reduce the likelihood of such incidents a bit also convert existing open coded mappings from buffer descriptors to buffer ids with BufferDescriptorGetBuffer(). Author: Qingqing Zhou Reported-By: Qingqing Zhou Discussion: CAJjS0u2ai9ooUisKtkV8cuVUtEkMTsbK8c7juNAjv8K11zeCQg@mail.gmail.com Back: 9.5 where the private ref count infrastructure was introduced
In some corner cases, it is possible for the BRIN index relation to be extended by brin_getinsertbuffer but the new page not be used immediately for anything by its callers; when this happens, the page is initialized and the FSM is updated (by brin_getinsertbuffer) with the info about that page, but these actions are not WAL-logged. A later index insert/update can use the page, but since the page is already initialized, the initialization itself is not WAL-logged then either. Replay of this sequence of events causes recovery to fail altogether. There is a related corner case within brin_getinsertbuffer itself, in which we extend the relation to put a new index tuple there, but later find out that we cannot do so, and do not return the buffer; the page obtained from extension is not even initialized. The resulting page is lost forever. To fix, shuffle the code so that initialization is not the responsibility of brin_getinsertbuffer anymore, in normal cases; instead, the initialization is done by its callers (brin_doinsert and brin_doupdate) once they're certain that the page is going to be used. When either those functions determine that the new page cannot be used, before bailing out they initialize the page as an empty regular page, enter it in FSM and WAL-log all this. This way, the page is usable for future index insertions, and WAL replay doesn't find trying to insert tuples in pages whose initialization didn't make it to the WAL. The same strategy is used in brin_getinsertbuffer when it cannot return the new page. Additionally, add a new step to vacuuming so that all pages of the index are scanned; whenever an uninitialized page is found, it is initialized as empty and WAL-logged. This closes the hole that the relation is extended but the system crashes before anything is WAL-logged about it. We also take this opportunity to update the FSM, in case it has gotten out of date. Thanks to Heikki Linnakangas for finding the problem that kicked some additional analysis of BRIN page assignment code. Back to 9.5, where BRIN was introduced. Discussion: https://www.postgresql.org/message-id/[email protected]
One of the changes I made in commit 8703059 turns out not to have been such a good idea: we still need the exception in join_is_legal() that allows a join if both inputs already overlap the RHS of the special join we're checking. Otherwise we can miss valid plans, and might indeed fail to find a plan at all, as in recent report from Andreas Seltenreich. That code was added way back in commit c171176, but I failed to include a regression test case then; my bad. Put it back with a better explanation, and a test this time. The logic does end up a bit different than before though: I now believe it's appropriate to make this check first, thereby allowing such a case whether or not we'd consider the previous SJ(s) to commute with this one. (Presumably, we already decided they did; but it was confusing to have this consideration in the middle of the code that was handling the other case.) Back- to all active branches, like the previous .
As complained by clang, reported by Andres Freund. Brown paper bag bug in ccc4c07. Add some comments, too. Back to 9.5, like that one.
Found and fixed by Andres Freund.
This function was using the single-value-per-call mechanism, but the code relied on a relcache entry that wasn't kept open across calls. This manifested as weird errors in buildfarm during the short time that the "brin-1" isolation test lived. Back to 9.5, where it was introduced.
In commit 95f4e59 I added a regression test case that examined the plan of a query on system catalogs. That isn't a terribly great idea because the catalogs tend to change from version to version, or even within a version if someone makes an unrelated regression-test change that populates the catalogs a bit differently. Usually I try to make planner test cases rely on test tables that have not changed since Berkeley days, but I got sloppy in this case because the submitted crasher example queried the catalogs and I didn't spend enough time on rewriting it. But it was a problem waiting to happen, as I was rudely reminded when I tried to port that into Salesforce's Postgres variant :-(. So spend a little more effort and rewrite the query to not use any system catalogs. I verified that this version still provokes the Assert if 95f4e59's code fix is reverted. I also removed the EXPLAIN output from the test, as it turns out that the assertion occurs while considering a plan that isn't the one ultimately selected anyway; so there's no value in risking any cross-platform variation in that printout. Back- to 9.2, like the previous .
This time, instead of using a core isolation test, put it on its own test module; this way it can require the pageinspect module to be present before running. The module's Makefile is loosely modeled after test_decoding's, so that it's easy to add further tests for either pg_regress or isolationtester later. Back to 9.5.
The build script is not able to parse the Makefile, so remove it.
Commit 49c817e replaced with a hard error the dubious pg_do_encoding_conversion() behavior when outside a transaction. Reintroduce the historic soft failure locally within pgwin32_message_to_UTF16(). This fixes errors when writing messages in less-common encodings to the Windows event log or console. Back- to 9.4, where the aforementioned commit first appeared. Per bug #13427 from Dmitri Bourlatchkov.
This fixes presentation of non-ASCII messages to the Windows event log and console in rare cases involving Korean locale. Processes like the postmaster and checkpointer, but not processes attached to databases, were affected. Back- to 9.4, where MessageEncoding was introduced. The problem exists in all supported versions, but this change has no effect in the absence of the code recognizing PG_UHC MessageEncoding. Noticed while investigating bug #13427 from Dmitri Bourlatchkov.
Mistakenly relreplident was stored as a bool. That works today as c.h typedefs bool to a char, but isn't very future proof. Discussion: [email protected] Back: 9.4 where replica identity was introduced.
Doing so doesn't work if bool is a macro rather than a typedef. Although c.h spends some effort to support configurations where bool is a preexisting macro, help_config.c has existed this way since 2003 (b700a6), and there have not been any reports of problems. Back anyway since this is as riskless as it gets. Discussion: [email protected] Back: 9.0-master
Since a179232 (vacuumdb: enable parallel mode) -1 has been assigned to a boolean. That can, justifiedly, trigger compiler warnings. There's also no need for ternary logic, result was only ever set to 0 or -1. So don't. Discussion: [email protected] Back: 9.5
It was a bool, even though it should be CEOUC_WAIT_MODE. That's unlikely to have a negative effect with the current definition of bool (char), but it's definitely wrong. Discussion: [email protected] Back: 9.5, where ON CONFLICT was merged
DO blocks use private simple_eval_estates to avoid intra-transaction memory age, cf commit c7b849a. I had forgotten about that while writing commit 0fc94a5, but it means that expression execution trees created within a DO block disappear immediately on exiting the DO block, and hence can't safely be linked into plpgsql's session-wide cast hash table. To fix, give a DO block a private cast hash table to go with its private simple_eval_estate. This is less efficient than one could wish, since DO blocks can no longer share any cast lookup work with other plpgsql execution, but it shouldn't be too bad; in any case it's no worse than what happened in DO blocks before commit 0fc94a5. Per bug #13571 from Feike Steenbergen. Preliminary analysis by Oleksandr Shulgin.
The table-rewriting forms of ALTER TABLE are MVCC-unsafe, in much the same way as TRUNCATE, because they replace all rows of the table with newly-made rows with a new xmin. (Ideally, concurrent transactions with old snapshots would continue to see the old table contents, but the data is not there anymore --- and if it were there, it would be inconsistent with the table's updated rowtype, so there would be serious implementation problems to fix.) This was nowhere documented though, and the problem was only documented for TRUNCATE in a note in the TRUNCATE reference page. Create a new "Caveats" section in the MVCC chapter that can be home to this and other limitations on serializable consistency. In passing, fix a mistaken statement that VACUUM and CLUSTER would reclaim space occupied by a dropped column. They don't reconstruct existing tuples so they couldn't do that. Back- to all supported branches.
This behavior wasn't documented, but it should be because it's user-visible in triggers and other functions executed on the remote server. Per question from Adam Fuchs. Back- to 9.3 where postgres_fdw was added.
With optimizations enabled at least one compiler, clang 3.7, optimized away the crc intrinsics knowing that the result went on unused and has no side effects. That can trigger errors in code generation when the intrinsic is used, as we chose to use the intrinsics without any additional compiler flag. Return the computed value to prevent that. With some more pedantic warning flags (-Wold-style-definition) the configure test failed to recognize the existence of _mm_crc32_u* intrinsics due to an independent warning in the test because the test turned on -Werror, but that's not actually needed here. Discussion: [email protected] Back: 9.5, where the use of crc intrinsics was integrated.
If the user has typed GRANT EXECUTE, the correct completion is "ON", not "PROCEDURE". Daniel Verite
plpgsql's error location context messages ("PL/pgSQL function fn-name line
line-no at stmt-type") would misreport a CONTINUE statement as being an
EXIT, and misreport a MOVE statement as being a FETCH. These are clear
bugs that have been there a long time, so back- to all supported
branches.
In addition, in 9.5 and HEAD, change the description of EXECUTE from
"EXECUTE statement" to just plain EXECUTE; there seems no good reason why
this statement type should be described differently from others that have
a well-defined head keyword. And distinguish GET STACKED DIAGNOSTICS from
plain GET DIAGNOSTICS. These are a bit more of a judgment call, and also
affect existing regression-test outputs, so I did not back- into
stable branches.
Pavel Stehule and Tom Lanewith suggestion from Michael Paquier
The shm_mq mechanism was intended to optionally notice when the process on the other end of the queue fails to attach to the queue. It does this by allowing the user to pass a BackgroundWorkerHandle; if the background worker in question is launched and dies without attaching to the queue, then we know it never will. This logic works OK in blocking mode, but when called with nowait = true we fail to notice that this has happened due to an asymmetry in the logic. Repair. Reported off-list by Rushabh Lathia. by me.
This way, we produce a better error message if someone tries to do something like ALTER INDEX .. ALTER COLUMN .. SET STORAGE. Amit Langote
If the counterparty writes some data into the queue and then detaches, it's wrong to return SHM_MQ_DETACHED right away. If we do that, we fail to read whatever was written.
Bernd Helmle complained that CreateReplicationSlot() was assigning the same value to the same variable twice, so we could remove one of them. Code inspection reveals that we can actually remove both assignments: according to the author the assignment was there for beauty of the strlen line only, and another possible fix to that is to put the strlen in its own line, so do that. To be consistent within the file, refactor all duplicated strlen() calls, which is what we do elsewhere in the backend anyway. In basebackup.c, snprintf already returns the right length; no need for strlen afterwards. Back to 9.4, where replication slots were introduced, to keep code identical. Some of this is older, but the doesn't apply cleanly and it's only of cosmetic value anyway. Discussion: http://www.postgresql.org/message-id/[email protected]
Further tweak commit_ts.c so that on a standby the state is completely consistent with what that in the master, rather than behaving differently in the cases that the settings differ. Now in standby and master the module should always be active or inactive in lockstep. Author: Petr Jelínek, with some further tweaks by Álvaro Herrera. Back to 9.5, where commit timestamps were introduced. Discussion: http://www.postgresql.org/message-id/[email protected]
A bug in the original free space computation made it possible to
return a page which wasn't actually able to fit the item. Since the
insertion code isn't prepared to deal with PageAddItem failing, a PANIC
resulted ("failed to add BRIN tuple [to new page]"). Add a macro to
encapsulate the correct computation, and use it in
brin_getinsertbuffer's callers before calling that routine, to raise an
early error.
I became aware of the possiblity of a problem in this area while working
on ccc4c07. There's no archived discussion about it, but it's
easy to reproduce a problem in the uned code with something like
CREATE TABLE t (a text);
CREATE INDEX ti ON t USING brin (a) WITH (pages_per_range=1);
for length in `seq 8000 8196`
do
psql -f - <<EOF
TRUNCATE TABLE t;
INSERT INTO t VALUES ('z'), (repeat('a', $length));
EOF
done
Back to 9.5, where BRIN was introduced.Back to 9.5 -- this should have been part of b0b7be6, but we didn't have 38b03ca either at the time. Author: Emre Hasegeli Revised by: Ian Barwick Discussion: http://www.postgresql.org/message-id/CAE2gYzyB39Q9up_-TO6FKhH44pcAM1x6n_Cuj15qKoLoFihUVg@mail.gmail.com http://www.postgresql.org/message-id/[email protected]
Per red wall in buildfarm
Mistake introduced by commit 3bf3ab8. Etsuro Fujita
Amit Langote, per Etsuro Fujita
Message style, plurals, quoting, spelling, consistency with similar messages
Show how this can be used in practice to make queries simpler and more flexible. Also, draw an explicit contrast to the existence operator, which doesn't work that way. Peter Geoghegan and Tom Lane
Mistake introduced by commit 5bd91e3. Hari Babu
On insert the CheckForSerializableConflictIn() test was performed before the page(s) which were going to be modified had been locked (with an exclusive buffer content lock). If another process acquired a relation SIReadLock on the heap and scanned to a page on which an insert was going to occur before the page was so locked, a rw-conflict would be missed, which could allow a serialization anomaly to be missed. The window between the check and the page lock was small, so the bug was generally not noticed unless there was high concurrency with multiple processes inserting into the same table. This was reported by Peter Bailis as bug #11732, by Sean Chittenden as bug #13667, and by others. The race condition was eliminated in heap_insert() by moving the check down below the acquisition of the buffer lock, which had been the very next statement. Because of the loop locking and unlocking multiple buffers in heap_multi_insert() a check was added after all inserts were completed. The check before the start of the inserts was left because it might avoid a large amount of work to detect a serialization anomaly before performing the all of the inserts and the related WAL logging. While investigating this bug, other SSI bugs which were even harder to hit in practice were noticed and fixed, an unnecessary check (covered by another check, so redundant) was removed from heap_update(), and comments were improved. Back- to all supported branches. Kevin Grittner and Thomas Munro
Back to 9.3, where it was initially omitted. Craig Ringer, with minor adjustment by Kevin Grittner
Commit a1480ec purported to fix the problems with commit b2ccb5f, but it didn't completely fix them. The problem is that the checks were performed in the wrong order, leading to a race condition. If the sender attached, sent a message, and detached after the receiver called shm_mq_get_sender and before the receiver called shm_mq_counterparty_gone, we'd incorrectly return SHM_MQ_DETACHED before all messages were read. Repair by reversing the order of operations, and add a long comment explaining why this new logic is (hopefully) correct.
Fix some brain fade in commit a2dabf0: erroneous variable names in docs, rearrangements that made sentences less clear not more so, undocumented and poorly-chosen-anyway API behaviors of subroutines, bad grammar in error messages, copy-and-paste faults. Albe Laurenz and Tom Lane
Standard-conforming literals have been the default for long enough that it no longer seems necessary to go out of our way to tell people to write regex escapes illegibly.
Peter Geoghegan
rafatower referenced this pull request in CartoDB/postgres Nov 23, 2016
When a subtransaction is aborted in plpython because of an SPI
exception, it tries to find a matching python exception in a hash
`PLy_spi_exceptions` and to make python vm raise it.
That hash is generated during module initialization, but the exception
objects are not marked to prevent the garbage collector from collecting
them, which can lead to a segmentation fault when processing any SPI
exception.
PoC to reproduce the issue:
```sql
CREATE OR REPLACE FUNCTION server_crashes()
RETURNS VOID
AS $$
import gc
gc.collect()
plan = plpy.prepare('SELECT raises_an_spi_exception();', [])
plpy.execute(plan)
$$ LANGUAGE plpythonu;
CREATE OR REPLACE FUNCTION raises_an_spi_exception()
RETURNS VOID
AS $$
DECLARE
sql TEXT;
BEGIN
sql = format('%I', NULL); -- NullValueNotAllowed
END
$$ LANGUAGE plpgsql;
SELECT server_crashes(); -- segfault here
```
Stacktrace of the problem (using PostgreSQL `REL9_5_STABLE` and python
`2.7.3-0ubuntu3.8` on a Ubuntu 12.04):
```
Program received signal SIGSEGV, Segmentation fault.
0x00007f3155c7670b in PyObject_Call (func=0x7f31b7db2a30, arg=0x7f31b87d17d0, kw=0x0) at ../Objects/abstract.c:2525
2525 ../Objects/abstract.c: No such file or directory.
(gdb) bt
#0 0x00007f3155c7670b in PyObject_Call (func=0x7f31b7db2a30, arg=0x7f31b87d17d0, kw=0x0) at ../Objects/abstract.c:2525
#1 0x00007f3155d81ab1 in PyEval_CallObjectWithKeywords (func=0x7f31b7db2a30, arg=0x7f31b87d17d0, kw=0x0) at ../Python/ceval.c:3890
#2 0x00007f3155c766ed in PyObject_CallObject (o=0x7f31b7db2a30, a=0x7f31b87d17d0) at ../Objects/abstract.c:2517
#3 0x00007f31561e112b in PLy_spi_exception_set (edata=0x7f31b8743d78, excclass=0x7f31b7db2a30) at plpy_spi.c:547
#4 PLy_spi_subtransaction_abort (oldcontext=<optimized out>, oldowner=<optimized out>) at plpy_spi.c:527
#5 0x00007f31561e2185 in PLy_spi_execute_plan (ob=0x7f31b87d0cd8, list=0x7f31b7c530d8, limit=0) at plpy_spi.c:307
#6 0x00007f31561e22d4 in PLy_spi_execute (self=<optimized out>, args=0x7f31b87a6d08) at plpy_spi.c:180
#7 0x00007f3155cda4d6 in PyCFunction_Call (func=0x7f31b7d29600, arg=0x7f31b87a6d08, kw=0x0) at ../Objects/methodobject.c:81
#8 0x00007f3155d82383 in call_function (pp_stack=0x7fff9207e710, oparg=2) at ../Python/ceval.c:4021
#9 0x00007f3155d7cda4 in PyEval_EvalFrameEx (f=0x7f31b8805be0, throwflag=0) at ../Python/ceval.c:2666
#10 0x00007f3155d82898 in fast_function (func=0x7f31b88b5ed0, pp_stack=0x7fff9207ea70, n=0, na=0, nk=0) at ../Python/ceval.c:4107
#11 0x00007f3155d82584 in call_function (pp_stack=0x7fff9207ea70, oparg=0) at ../Python/ceval.c:4042
#12 0x00007f3155d7cda4 in PyEval_EvalFrameEx (f=0x7f31b8805a00, throwflag=0) at ../Python/ceval.c:2666
#13 0x00007f3155d7f8a9 in PyEval_EvalCodeEx (co=0x7f31b88aa460, globals=0x7f31b8727ea0, locals=0x7f31b8727ea0, args=0x0, argcount=0, kws=0x0, kwcount=0, defs=0x0, defcount=0, closure=0x0) at ../Python/ceval.c:3253
#14 0x00007f3155d74ff4 in PyEval_EvalCode (co=0x7f31b88aa460, globals=0x7f31b8727ea0, locals=0x7f31b8727ea0) at ../Python/ceval.c:667
#15 0x00007f31561dc476 in PLy_procedure_call (kargs=kargs@entry=0x7f31561e5690 "args", vargs=<optimized out>, proc=0x7f31b873b2d0, proc=0x7f31b873b2d0) at plpy_exec.c:801
#16 0x00007f31561dd9c6 in PLy_exec_function (fcinfo=fcinfo@entry=0x7f31b7c1f870, proc=0x7f31b873b2d0) at plpy_exec.c:61#17 0x00007f31561de9f9 in plpython_call_handler (fcinfo=0x7f31b7c1f870) at plpy_main.c:291
```Thanks for your Pull Request! 😄 This repo on is just a mirror of our real git repositories though, and can't really handle PRs. 😦 Hopefully you can redo the PR, and direct it to the git.postgresql.org repos? We have a developer guide, if that helps: https://wiki.postgresql.org/wiki/So,_you_want_to_be_a_developer%3F. If this was a PR for pgAdmin, please visit https://www.pgadmin.org/docs/pgadmin4/dev/submitting_es.html. |
zhihuiFan pushed a commit to zhihuiFan/postgres that referenced this pull request Jul 26, 2021
Authored by @japinli, with added quoting by me. Fixes postgres#9. Co-authored-by: Japin <[email protected]>
petere pushed a commit to petere/postgresql that referenced this pull request Jun 8, 2022
justinpryzby pushed a commit to justinpryzby/postgres that referenced this pull request Jun 13, 2022
justinpryzby pushed a commit to justinpryzby/postgres that referenced this pull request Jun 13, 2022
justinpryzby pushed a commit to justinpryzby/postgres that referenced this pull request Jun 13, 2022
Sign up for free to subscribe to this conversation on . Already have an account? Sign in.
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
No description provided.