Skip to content

Conversation

@aprimadi
Copy link
Contributor

@aprimadi aprimadi commented May 15, 2023

Which issue does this PR close?

Closes #6251

Rationale for this change

N/A

What changes are included in this PR?

  • Refactor create external table parser to use parse_one_of_keywords
  • Add support for CREATE UNBOUNDED EXTERNAL TABLE ... statement

Are these changes tested?

Yes

Are there any user-facing changes?

No API change. Add more SQL functionality.

@github-actions github-actions bot added the sql SQL Planner label May 15, 2023
@aprimadi aprimadi changed the title Refactor create external table to use one_of_keywords Support CREATE TABLE via SQL for infinite streams May 15, 2023
@github-actions github-actions bot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels May 15, 2023
@github-actions github-actions bot added the logical-expr Logical plan and expressions label May 15, 2023
@alamb
Copy link
Contributor

alamb commented May 15, 2023

cc @ozankabak and @mustafasrepo

@aprimadi aprimadi marked this pull request as ready for review May 16, 2023 03:43
/// File compression type (GZIP, BZIP2, XZ, ZSTD)
pub file_compression_type: CompressionTypeVariant,
/// Whether the table is an infinite streams
pub unbounded: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, fn hash<H: Hasher>(&self, state: &mut H) function should use self.unbounded during hashing also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, I miss that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you

self.parser.expect_keyword(Keyword::ROW)?;
ensure_not_set(&builder.has_header, "WITH HEADER ROW")?;
builder.has_header = Some(true);
if let Some(keyword) = self.parser.parse_one_of_keywords(&[
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice cleanup!.

@mustafasrepo
Copy link
Contributor

This PR is LGTM!. Thanks @aprimadi for this work.

mustafasrepo
mustafasrepo previously approved these changes May 16, 2023
@mustafasrepo mustafasrepo dismissed their stale review May 16, 2023 10:29

After unbounded information is used in hash function I will approve this PR. Someone may see this PR approved, then may merge this, before that feature is addressed.

@aprimadi
Copy link
Contributor Author

Thank you for the review @mustafasrepo

@alamb
Copy link
Contributor

alamb commented May 16, 2023

Thanks @aprimadi and @mustafasrepo !

@alamb
Copy link
Contributor

alamb commented May 16, 2023

🤔 github seems to be acting up (not merging this PR).

@aprimadi
Copy link
Contributor Author

@alamb is it supposed to merge automatically?

@mustafasrepo mustafasrepo merged commit 20c7af3 into apache:main May 17, 2023
@aprimadi aprimadi deleted the parser-create-unbounded branch May 17, 2023 06:44
@alamb alamb mentioned this pull request May 17, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate logical-expr Logical plan and expressions sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support CREATE TABLE via SQL for infinite streams

3 participants