Integrate catalog schema validation into planner -WIP #15711
Conversation
github-actions
bot
added
Area - Batch Ingestion
Area - Querying
Area - MSQ
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
sql/src/main/java/org/apache/druid/sql/calcite/planner/CalcitePlanner.java
Fixed
Show resolved
Hide resolved
sql/src/test/java/org/apache/druid/sql/calcite/planner/PlannerCaptureHook.java
Fixed
Show fixed
Hide fixed
| * Instead return what would have been sent to the execution engine. | ||
| * The result is a Jackson-serializable query plan. | ||
| */ | ||
| default Object explain(DruidQuery druidQuery) |
Check notice
Code scanning / CodeQL
Useless parameter Note
| @@ -94,6 +94,7 @@ | |||
| QueryMaker buildQueryMakerForInsert( | |||
| String targetDataSource, | |||
| RelRoot relRoot, | |||
| PlannerContext plannerContext | |||
| PlannerContext plannerContext, | |||
| RelDataType targetType | |||
Check notice
Code scanning / CodeQL
Useless parameter Note
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
Show resolved
Hide resolved
| } | ||
| { | ||
| ( | ||
| <HOUR> | ||
| { | ||
| granularity = Granularities.HOUR; | ||
| unparseString = "HOUR"; | ||
| result = SqlLiteral.createCharString(DruidSqlParserUtils.HOUR_GRAIN, getPos()); |
There was a problem hiding this comment.
I wonder if it would be possible to use SqlLiteral.createSymbol here instead; that could remove the need for the string based matching as well...
There was a problem hiding this comment.
using SqlLiteral.createSymbol as you suggested
| // Add the necessary indirection. The type factory used here | ||
| // is the Druid one, since the per-query one is not yet available | ||
| // here. Nor are built-in function associated with per-query types. | ||
| this.operatorTable = new ChainedSqlOperatorTable( |
There was a problem hiding this comment.
I wonder if this is new functionality - could it be in a separate PR?
There was a problem hiding this comment.
Thanks! removed this
| @@ -58,7 +58,7 @@ public void testUnparseReplaceAll() throws ParseException | |||
| + "OVERWRITE ALL\n" | |||
| + "SELECT *\n" | |||
| + " FROM \"foo\"\n" | |||
| + "PARTITIONED BY ALL TIME " | |||
| + "PARTITIONED BY 'ALL TIME' " | |||
There was a problem hiding this comment.
is the result of the unparse valid ?
* fix ingest on non-catalog table failure
|
This pull request has been marked as stale due to 60 days of inactivity. |
|
This pull request/issue has been closed due to lack of activity. If you think that |
Description
This PR contains a portion of the changes from the inactive draft PR for integrating the catalog with the Calcite planner #13686 from @paul-rogers, allowing the datasource table schemas defined in the catalog to be validated against when ingesting data into the underlying datasource, during SQL based ingestion. This allows for SQL based ingestion into a table with a declared schema to produce segments that conform to that schema. If partitioning and clustering is not defined at ingestion time, defaults for these parameters, as defined in the catalog for the table, if found, are used.
TODO: add more tests.
Release note
Key changed/added classes in this PR
MyFooOurBarTheirBazThis PR has: