Skip to content

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::Optional(SyntaxKind::Or),
SyntaxToken::Optional(SyntaxKind::Replace),
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

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
@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