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: support create type #118

Merged
merged 2 commits into from
Feb 8, 2024
Merged

Conversation

cvng
Copy link
Contributor

@cvng cvng commented Jan 24, 2024

What kind of change does this PR introduce?

add support for object type in DefineStmt

What is the current behavior?

--

What is the new behavior?

--

Additional context

while working on this, I realized there is a few statements that have an AST mismatch from pg_query:

https://github.com/search?q=repo%3Asupabase%2Fpostgres_lsp+%22stmts%3A+%5B%5D%2C%22&type=code

with either: no AST (sometimes expected) or AST with wrong nodes (eg. for create procedure)

@psteinroe lmk if i should try to investigate these ones next

}

#[test]
fn test_create_composite_type() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: create type as constitute a composite type

@psteinroe
Copy link
Collaborator

@cvng thanks! yes, there are some that do not work yet. I have already created an issue to investigate. if you like, please go ahead and check them.

SyntaxToken::Required(SyntaxKind::TypeP),
SyntaxToken::Required(SyntaxKind::Ident),
Copy link
Contributor Author

@cvng cvng Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fix part for 6f8418b is here: DefineStmt was missing SyntaxKind::Ident

otherwise I have removed some illegal tokens for some statements (eg. or replace) based on the docs

@@ -1,6 +1,6 @@
CREATE TYPE type1;
CREATE TYPE type1 AS (attr1 int4, attr2 bool);
CREATE TYPE type1 AS (attr1 int4 COLLATE collation1, attr2 bool);
/* TODO: CREATE TYPE type1 AS (attr1 int4 COLLATE collation1, attr2 bool); */ SELECT 1;
Copy link
Contributor Author

@cvng cvng Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't get COLLATE to work here, I have tried CollateClause ColumnDef CompositeTypeStmt with no luck

it is obfuscated in the macro stack but we have a hint that it panics in codegen::get_location_internal

,
errors: [],
stmts: [],
stmts: [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@psteinroe
Copy link
Collaborator

awesome!

@psteinroe psteinroe merged commit efb2e9f into supabase-community:main Feb 8, 2024
1 check passed
@cvng cvng deleted the fix/define-stmt branch February 10, 2024 13:30
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