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

feat: support create conversion #97

Merged
merged 2 commits into from
Dec 28, 2023

Conversation

jasonpanosso
Copy link
Contributor

What kind of change does this PR introduce?

Support create conversion, chip away at #51

What is the current behavior?

LSP quits with exit code 101 upon attempting to parse create conversion

LSP Log

[START][2023-12-27 10:41:20] LSP logging initiated
[ERROR][2023-12-27 10:41:20] .../vim/lsp/rpc.lua:675 "rpc" "/home/jason/.cargo/bin/postgres_lsp" "stderr" "thread 'main' panicked at crates/parser/src/parse/libpg_query_node.rs:148:17: could not find node for token Token { kind: Create, text: "CREATE", span: 0..6, token_type: ReservedKeyword } at depth 1
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace"

What is the new behavior?

Handles create conversion queries

Additional context

Is there a file naming convention for the tests in crates/parser/tests/data/statements? I see some gaps in the numbers, but I couldn't identify a pattern so I just added a new test at the end.

Also, not sure if checking for n.func_name.len() > 0 is the best approach for handling adding the From token, as I don't understand why func_name is a List*. Not sure when it would ever not be 0 or 1, would seriously appreciate some insight :)

Awesome project, thx so much for the work!

@psteinroe
Copy link
Collaborator

hey @jasonpanosso, thanks for the contribution!

Is there a file naming convention for the tests in crates/parser/tests/data/statements? I see some gaps in the numbers, but I couldn't identify a pattern so I just added a new test at the end.

nope, the gaps just occurred because of pull requests being merged in different orders. you can name them however you like. :)

Also, not sure if checking for n.func_name.len() > 0 is the best approach for handling adding the From token, as I don't understand why func_name is a List*

I honestly have no idea. The postgres docs pretty clearly show just a single function. Could you maybe add an extra check and panic of the length of the vector is > 1?

@jasonpanosso
Copy link
Contributor Author

nope, the gaps just occurred because of pull requests being merged in different orders. you can name them however you like. :)

Makes sense!

I honestly have no idea. The postgres docs pretty clearly show just a single function. Could you maybe add an extra check and panic of the length of the vector is > 1?

Strange, must be some weird internal pg stuff.. I pushed a fix that panics when the length of the vec is > 1

@psteinroe psteinroe merged commit d7df5d5 into supabase-community:main Dec 28, 2023
1 check passed
@jasonpanosso jasonpanosso deleted the jp/create_conversion branch December 29, 2023 16:22
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