Skip to content

Snowflake#93

Open
not-night-but wants to merge 4 commits into
mainfrom
feat/snowflake
Open

Snowflake#93
not-night-but wants to merge 4 commits into
mainfrom
feat/snowflake

Conversation

@not-night-but
Copy link
Copy Markdown
Contributor

Snowflake implementation. Also adds some keywords/statements that seemed to be missing (USE, COPY, MERGE, etc)

Copy link
Copy Markdown
Contributor

@rathboma rathboma left a comment

Choose a reason for hiding this comment

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

I think we probably have to deal with the Snowflake keywords leaking into other dialects

Comment thread src/tokenizer.ts
'TRIGGERS',
'VARIABLES',
'WARNINGS',
'MERGE',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are these keywords valid for every dialect? Should we be scoping these somehow?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like the addition of LIST even required a test change?

Comment thread src/tokenizer.ts
'PUT',
'GET',
'LIST',
'LS',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

certainly feels like we might need to keep snowflake-specific keywords out of the global tokenizer list and scope them based on dialect

Comment thread src/parser.ts
return createBlockStatementParser(options);
}
break;
case 'MERGE':
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are any of these Snowflake specific? Looks like we dialect-gate some below, but not these presumably because they're widely supported?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure that PartiQL doesn't support a lot of these for example

describe('with strict disabled', () => {
it('should parse if first token is unknown', () => {
const actual = parse('LIST * FROM foo', false);
const actual = parse('FOOBAR * FROM foo', false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see we had to change it! This is invalid SQL in a bunch of dialects I think. Don't think LIST is AnsiSQL or supported by postgresql, and I don't think MySQL either?

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