Conversation
The C branch in _pre_process never matched: it had no single-string form, and the link symbol had already been replaced by an AAKEYWCONST placeholder. Since SQL function bodies are formatted, the strings were then reformatted as SQL, changing the library PostgreSQL loads.
|
I have not reviewed this patch but +1 fixing this. Would love to incorporate pgformatter into extension CI pipelines to enforce formatting but am not able to because of this issue. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Since SQL function bodies are formatted (eaa0529), the string after
ASin aLANGUAGE Cfunction is reformatted as SQL:and
AS 'MODULE_PATHNAME'becomes'\n MODULE_PATHNAME;\n', which changes the library PostgreSQL loads. The C branch in_pre_processthat should hide these strings never matched: it had no single-string form, and the link symbol had already been replaced by anAAKEYWCONSTplaceholder. It now accepts both forms, so the strings are kept verbatim with LANGUAGE before or after AS.expected/ex61.sqlgoes back to its output before the regression (identical to 0707b83), and the new ex90 covers both C forms plus aLANGUAGE sqlbody that is still formatted. Formatting contrib hstore 1.4 now leaves all 57 C function definitions with the same parse tree (checked with pglast).prove -l t/andt/regress_test.plpass. This also bumpst/02_regress.tto 91, like #418 and #419, so whichever lands later needs the count adjusted.Single-quoted bodies of other non-SQL languages are also formatted (
AS 'return 1;' LANGUAGE plperlbecomesRETURN 1;); I left that out here and can send it separately if you want it handled.Written with AI assistance (Claude); I have reviewed the change.