Minor: add flags for temporary ddl #12561
Conversation
github-actions
bot
added
sql
hailelagi
force-pushed
the
haile/add-flag-for-temporary-ddl
branch
from
0e03cba
to
a023c22
Compare
|
sorry, i took a bit longer than i was hoping with this one as i tried to get it to require minimal changes in the diff but as it is an api change i ended up spending quite a lot of time reading code. I have some questions and i'm super open to feedback, i'll self-review a few places where i was especially confused |
github-actions
bot
added
core
hailelagi
force-pushed
the
haile/add-flag-for-temporary-ddl
branch
from
a023c22
to
2cb1e93
Compare
hailelagi
changed the title
There was a problem hiding this comment.
Thanks @hailelagi . I think we should add some sqllogictest tests for this change. Perhaps in ddl.slt. In order to ensure the generation of correct logical plans, we can add tests similar to the following.
set datafusion.explain.logical_plan_only=true;
explain create temp table t(a int);
datafusion/sql/src/parser.rs
Outdated
| @@ -704,6 +706,10 @@ impl<'a> DFParser<'a> { | |||
| self.parser | |||
| .parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); | |||
| let table_name = self.parser.parse_object_name(true)?; | |||
| let temporary = self | |||
There was a problem hiding this comment.
I am not sure if we need the temporary flag for an external table, as sqlparser-rs does not support it.
If we want to support it, the syntax is best as create temp external table table_name ... or create external table temp table_name ... .
When specifying columns, the current syntax is not user-friendly.
create external table table_name temporary(a int) STORED AS csv location '/tmp';
There was a problem hiding this comment.
would it make sense to create an issue in sqlparser-rs for this? @jonahgao ?
Signed-off-by: Haile Lagi <[email protected]>
hailelagi
force-pushed
the
haile/add-flag-for-temporary-ddl
branch
from
2cb1e93
to
7ac494c
Compare
Which issue does this PR close?
Closes #12463
Rationale for this change
This introduces a new flag option
temporaryon theDdlStatementof:CreateMemoryTable,CreateExternalTableThese flags enable an implementation built on DF to define the semantics of a temporary table.
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?