Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix some plpgsql errors #244

Closed
wants to merge 8 commits into from
Closed

Conversation

MGorkov
Copy link

@MGorkov MGorkov commented Apr 16, 2024

This patch fixes the #243 , #160 , #248 issues and the following errors:
"variable "$1" does not exist"
"collations are not supported by type -"
"variable "ref" must be of type cursor or refcursor"
missing expression at or near ";"

changes in JSON:

  1. add declared variable with "alliasfor" parameter pointing to associated variable, e.g. DECLARE _a ALIAS FOR $1;
    {
      PLpgSQL_var: {
        refname: '_a',
        aliasfor: '$1',
        lineno: 3,
        datatype: { PLpgSQL_type: { typname: 'text' } }
      }
  1. add "retvarno" parameter for RETURN NEXT statement
    {
      PLpgSQL_stmt_return_next: { lineno: 6, retvarno: 2 }
    }

@MGorkov MGorkov changed the title fix ival for negatives and zeroes fix ival for negatives and zeros Apr 16, 2024
@lfittl
Copy link
Member

lfittl commented Apr 26, 2024

Thanks for the patch, and conceptually looks good!

However it appears there is a problem with the protobuf-cpp tests here, blocking a merge.

I believe this is actually a larger issue with how we handle oneof in our JSON output, I started fixing that in #246, pulling in your commit there as well. Assuming I can work out the open issues on that PR I'll close this here and fix it by merging the other PR.

@MGorkov MGorkov force-pushed the master branch 3 times, most recently from 53a97cc to 56d91d3 Compare May 7, 2024 14:58
- variable "$1" does not exist
- collations are not supported by type -
- variable "ref" must be of type cursor or refcursor
@MGorkov MGorkov changed the title fix ival for negatives and zeros fix some plpgsql errors May 8, 2024
msullivan added a commit to msullivan/libpg_query that referenced this pull request May 9, 2024
Currently the value of negative constants are omitted. Only omit *0*
constants.

An earlier attempt (pganalyze#244) caused a mismatch with the protobuf json
output, since that wants to omit values that are equal to the default
value.

I don't *really* think that is particularly sensible behavior for an
integer constant in an AST, but oh well.

Fixes pganalyze#243.
msullivan added a commit to msullivan/libpg_query that referenced this pull request May 9, 2024
Currently the value of negative constants are omitted. Only omit *0*
constants.

An earlier attempt (pganalyze#244) caused a mismatch with the protobuf json
output, since that wants to omit values that are equal to the default
value.

I don't *really* think that is particularly sensible behavior for an
integer constant in an AST, but oh well.

Fixes pganalyze#243.
msullivan added a commit to msullivan/libpg_query that referenced this pull request May 9, 2024
Currently the value of negative constants are omitted. Only omit *0*
constants.

An earlier attempt (pganalyze#244) caused a mismatch with the protobuf json
output, since that wants to omit values that are equal to the default
value.

I don't *really* think that is particularly sensible behavior for an
integer constant in an AST, but oh well.

Fixes pganalyze#243.
@lfittl
Copy link
Member

lfittl commented Oct 9, 2024

@MGorkov Thanks for your contributions in this PR! I ended up reworking them a bit and merging them in #263.

I did not take all changes, but tried to keep it more narrow to the parts that looked most relevant to make parsing work in cases where it didn't work before.

If there are additional changes you'd like to see, could you propose these in a new PR? I'll close this here for now, since I believe the main issues are addressed.

@lfittl lfittl closed this Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants