-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Introduce dynamic table append #15897
Conversation
processing/src/main/java/org/apache/druid/segment/column/RowSignature.java
Fixed
Show fixed
Hide fixed
sql/src/main/java/org/apache/druid/sql/calcite/external/ConcatTableMacro.java
Fixed
Show fixed
Hide fixed
sql/src/main/java/org/apache/druid/sql/calcite/external/ConcatTableMacro.java
Fixed
Show fixed
Hide fixed
sql/src/main/java/org/apache/druid/sql/calcite/external/ConcatTableMacro.java
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for the changes @kgyrtkirk :)
I have left a few comments on the changes, please let me know your thoughts
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Show resolved
Hide resolved
sql/src/test/java/org/apache/druid/sql/calcite/CalciteTableAppendTest.java
Outdated
Show resolved
Hide resolved
sql/src/test/java/org/apache/druid/sql/calcite/CalciteTableAppendTest.java
Show resolved
Hide resolved
sql/src/test/java/org/apache/druid/sql/calcite/CalciteTableAppendTest.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/AppendTableMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/table/DatasourceMetadata.java
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Outdated
Show resolved
Hide resolved
sql/src/main/java/org/apache/druid/sql/calcite/external/TableAppendMacro.java
Show resolved
Hide resolved
sql/src/test/java/org/apache/druid/sql/calcite/CalciteTableAppendTest.java
Outdated
Show resolved
Hide resolved
sql/src/test/java/org/apache/druid/sql/calcite/CalciteTableAppendTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new docs look great. I left some suggestions to improve scanning and readability.
docs/querying/datasource.md
Outdated
@@ -211,6 +219,50 @@ Inline datasources are not available in Druid SQL. | |||
Refer to the [Query execution](query-execution.md#inline) page for more details on how queries are executed when you | |||
use inline datasources. | |||
|
|||
### Dynamic table append `TABLE(APPEND(...))` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Since this has the same native query syntax as
union
should this content go within the existingunion
section? - Suggest removing the SQL syntax from the header title, so just have
#### Dynamic table append
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved the section to be the next after union
what do you think:
- should this be made a subsection of
union
? - should I remove the
native
as that matches the standard union's behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like both of those ideas. Since the native union behavior is the same, the new content can go in there, and removing the repeated native
section won't have people wondering what the difference is.
Co-authored-by: Victoria Lim <vtlim@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment on heading styling. With that and the changes in #15897 (comment), docs should be all set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Dynamic table appaned
TABLE(APPEND(...))
This is essentially an SQL level syntactic sugar which matches columns by name from multiple tables.
Suppose you have 3 tables:
table1
hascolumn1
table2
hascolumn2
table3
hascolumn1
,column2
,column3
It is possible to create an union view of the above tables is possible by using UNION ALL:
However depending on the size of the table's schema it might be quite complicated to do that;
TABLE(APPEND('table1','table2','table3'))
represents the same in a more compact form.It adds one additional thing
ColumnType.leastRestrictive
is used to get a common type for themRelease note
Introduce dynamic table append -
SELECT * FROM TABLE(APPEND('table1','table2'))
implicitly creates an union based on the table schemas.This PR has: