peter-toth commented on code in PR #58153:
URL: https://github.com/apache/spark/pull/58153#discussion_r3824585772
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowCreateTableExec.scala:
##########
@@ -125,6 +126,45 @@ case class ShowCreateTableExec(
}
}
+ /**
+ * Emits the write distribution and ordering the table declares as the
default for writes into it,
+ * so that a table created with those clauses can be recreated from this
statement.
+ *
+ * The pair a connector may report is wider than the syntax can spell:
`hash` on a table with no
+ * partitioning (the parser rejects `DISTRIBUTED BY PARTITION` there), a
`range` distribution with
+ * no ordering, an ordering with no distribution, and a mode this Spark
version does not know.
+ * Those are left out rather than guessed at, since emitting a clause that
means something else --
+ * or one that does not parse at all -- would be worse than emitting none.
DESCRIBE TABLE EXTENDED
+ * reports both values verbatim regardless.
+ */
+ private def showTableWriteDistributionAndOrdering(
+ table: Table,
+ builder: StringBuilder): Unit = {
+ val orderBy = if (table.writeOrdering().nonEmpty) {
+ Some(table.writeOrdering()
+ .map(WriteDistributionAndOrdering.describeSortOrder)
+ .mkString("ORDERED BY (", ", ", ")"))
Review Comment:
**Finding 2.** The `hasPartitioning` guard below covers the distribution
side of this method's own doc - "emitting a clause that means something else --
or one that does not parse at all -- would be worse than emitting none" - but
nothing checks that the sort expressions are spellable.
`SortOrder.expression()` is typed `Expression` and `Table.writeOrdering()`'s
new contract does not narrow it, so a connector may report an expression the
`writeOrderField : transform ...` rule cannot represent.
Measured on this branch, with a table reporting mode `range` and `sort(a +
1, ASCENDING, NULLS_FIRST)`:
CREATE TABLE p.t (
id INT)
USING foo
ORDERED BY (id + 1 ASC NULLS FIRST)
Replaying that gives `[PARSE_SYNTAX_ERROR] Syntax error at or near '+'` at
line 4, pos 15. The whole statement is unrunnable, not one clause lost, which
is the worse failure the doc calls out.
Same fix shape as the `hash` case: drop the pair when it cannot be spelled.
Two things to get right. `CreateTableWriteOrderSuite` builds a bare
`FieldReference`, not a `Transform`, and that one does round-trip, so the
predicate has to admit it. And nulling out `orderBy` alone is not enough -
`(none, None)` would then emit `UNORDERED`, declaring no ordering on a table
that has one - so the whole method has to bail:
```scala
private def isSpellable(e: V2Expression): Boolean = e match {
case _: NamedReference => true
// Mirrors `transformArgument : qualifiedName | constant`.
case t: Transform =>
t.arguments().forall(a => a.isInstanceOf[NamedReference] ||
a.isInstanceOf[Literal[_]])
case _ => false
}
```
then wrap the existing body in `if (table.writeOrdering().forall(o =>
isSpellable(o.expression()))) { ... }`. `DESCRIBE TABLE EXTENDED` still reports
both values verbatim, so nothing is hidden.
##########
docs/sql-ref-syntax-ddl-create-table-datasource.md:
##########
@@ -98,6 +106,60 @@ as any order. For example, you can write COMMENT
table_comment after TBLPROPERTI
A list of key-value pairs that is used to tag the table definition.
+* **DISTRIBUTED BY PARTITION**
+
+ Requests that every write to the table be clustered by the table's
partitioning, so each
+ partition is written by a single task rather than by every task that holds
rows for it.
+
+ Requires the table to actually be partitioned, with `PARTITIONED BY` or
with
+ `CLUSTERED BY ... INTO ... BUCKETS`. Note that `CLUSTER BY` -- a different
clause from
+ `CLUSTERED BY ... INTO ... BUCKETS` -- does **not** qualify: it lists
clustering columns for the
+ data source to interpret rather than defining a partitioning, and it
cannot be combined with
+ `PARTITIONED BY` or `CLUSTERED BY ... INTO ... BUCKETS`, so a table using
it has no partitioning
+ to distribute by.
+
+* **ORDERED BY**
+
+ Requests a sort order for every write to the table, recorded on the table
so that later writes
+ honor it too. `UNORDERED` asks for no ordering at all, which is different
from omitting the
+ clause -- omitting it leaves the choice to the data source. The
parentheses are optional:
+ `ORDERED BY (a, b)` and `ORDERED BY a, b` are the same. The sort keys must
resolve against the
+ table's columns, so a `CREATE TABLE` with neither a column list nor `AS
SELECT` cannot use this
+ clause.
+
+ The distribution decides how far the order reaches, and this clause picks
one when
+ `DISTRIBUTED BY PARTITION` is absent: a bare `ORDERED BY` range-partitions
each write, so the
+ order holds across the whole table, while `LOCALLY ORDERED BY` asks for it
to hold within each
+ written file only, without a shuffle. `UNORDERED` on its own asks for no
distribution either.
+
+ When `DISTRIBUTED BY PARTITION` is given it decides the distribution
instead, and the order then
+ holds within each write task. `LOCALLY` therefore adds nothing beside it,
and `UNORDERED` beside
+ it contributes only "no sort keys":
+
+ ```sql
+ -- range-partition each write by id, so the order holds across the whole
table
+ CREATE TABLE t (id INT, c STRING) USING iceberg PARTITIONED BY (c) ORDERED
BY (id);
+
+ -- cluster each write by partition instead, and sort by id within each task
+ CREATE TABLE t (id INT, c STRING) USING iceberg PARTITIONED BY (c)
+ DISTRIBUTED BY PARTITION ORDERED BY (id);
+
+ -- cluster each write by partition, with no sort order; UNORDERED here is
the same as
+ -- omitting it, since DISTRIBUTED BY PARTITION already fixed the
distribution
+ CREATE TABLE t (id INT, c STRING) USING iceberg PARTITIONED BY (c)
+ DISTRIBUTED BY PARTITION UNORDERED;
+ ```
+
+ Both clauses are passed to the data source, which has to support them: a
data source that does
Review Comment:
**Finding 4.** The advertiser is the catalog, not the data source. What
gates this is `TableCatalog.capabilities()` returning
`TableCatalogCapability.SUPPORTS_CREATE_TABLE_WITH_WRITE_DISTRIBUTION_AND_ORDERING`,
checked in
`WriteDistributionAndOrdering.validateCatalogForWriteDistributionAndOrdering`.
A user who hits `UNSUPPORTED_FEATURE.TABLE_OPERATION` and follows this
paragraph will inspect the `USING` provider, where there is nothing to inspect.
It also makes the last sentence hard to act on. "The built-in data sources
do not support them" is true, but the reason is that `V2SessionCatalog` does
not advertise the capability and the v1 conversion in `ResolveSessionCatalog`
rejects the request outright - nothing to do with parquet or ORC.
Suggest saying catalog throughout: "Both clauses are passed to the catalog,
which has to support them: a catalog that does not advertise support ... The
built-in catalogs do not."
##########
docs/sql-ref-syntax-ddl-create-table-datasource.md:
##########
@@ -98,6 +106,60 @@ as any order. For example, you can write COMMENT
table_comment after TBLPROPERTI
A list of key-value pairs that is used to tag the table definition.
+* **DISTRIBUTED BY PARTITION**
+
+ Requests that every write to the table be clustered by the table's
partitioning, so each
+ partition is written by a single task rather than by every task that holds
rows for it.
+
+ Requires the table to actually be partitioned, with `PARTITIONED BY` or
with
+ `CLUSTERED BY ... INTO ... BUCKETS`. Note that `CLUSTER BY` -- a different
clause from
+ `CLUSTERED BY ... INTO ... BUCKETS` -- does **not** qualify: it lists
clustering columns for the
+ data source to interpret rather than defining a partitioning, and it
cannot be combined with
+ `PARTITIONED BY` or `CLUSTERED BY ... INTO ... BUCKETS`, so a table using
it has no partitioning
+ to distribute by.
+
+* **ORDERED BY**
+
+ Requests a sort order for every write to the table, recorded on the table
so that later writes
+ honor it too. `UNORDERED` asks for no ordering at all, which is different
from omitting the
+ clause -- omitting it leaves the choice to the data source. The
parentheses are optional:
+ `ORDERED BY (a, b)` and `ORDERED BY a, b` are the same. The sort keys must
resolve against the
+ table's columns, so a `CREATE TABLE` with neither a column list nor `AS
SELECT` cannot use this
+ clause.
+
+ The distribution decides how far the order reaches, and this clause picks
one when
+ `DISTRIBUTED BY PARTITION` is absent: a bare `ORDERED BY` range-partitions
each write, so the
+ order holds across the whole table, while `LOCALLY ORDERED BY` asks for it
to hold within each
+ written file only, without a shuffle. `UNORDERED` on its own asks for no
distribution either.
Review Comment:
**Finding 6.** "within each written file only" here, "within each write
task" three lines down at :136. A task can roll over several files, so these
are different claims, and the second is the accurate one - it also matches
`TableInfo.DISTRIBUTION_MODE_NONE`'s javadoc ("any ordering holds within a
write task only"). `DISTRIBUTION_MODE_RANGE`'s javadoc has the same drift the
other way ("across files, not only within one").
##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -9137,6 +9151,11 @@
"message" : [
"Write for the binary file data source."
]
+ },
+ "WRITE_ORDERING_WITH_NESTED_COLUMN_IS_UNSUPPORTED" : {
Review Comment:
**Finding 3.** Both the name and the message describe cases this condition
cannot report.
A nested *struct* column is supported, and the suite asserts it - `CREATE
TABLE testcat.t (p STRUCT<x: INT>) USING foo ORDERED BY p.x` is accepted. So
`WITH_NESTED_COLUMN_IS_UNSUPPORTED` says the opposite of the tested behaviour.
"or is in a map or array" is unreachable. `StructType.findNestedField` is
called with the default `includeCollections = false`, which *throws*
`INVALID_FIELD_NAME` on a path through a non-struct rather than returning
`None`. Measured on this branch:
ORDERED BY (m.key) on MAP<STRING, INT>
-> [INVALID_FIELD_NAME] Field name `m`.`key` is invalid: `m` is not a
struct
ORDERED BY (a.element.x) on ARRAY<STRUCT<x: INT>>
-> [INVALID_FIELD_NAME] Field name `a`.`element`.`x` is invalid: `a`
is not a struct
ORDERED BY (truncate(4, m.key)) on MAP<STRING, INT>
-> [INVALID_FIELD_NAME] Field name `m`.`key` is invalid: `m` is not a
struct
The third one is raised from the `CheckAnalysis` check itself:
`truncate(...)` is an `ApplyTransform`, so it is not rewritable and never
reaches `PreprocessTableCreation`.
What this condition actually reports is a reference that is not a column of
the table. I know it is a faithful copy of
`PARTITION_WITH_NESTED_COLUMN_IS_UNSUPPORTED`, where both faults are equally
present, and consistency with the sibling is a real argument - it just does not
carry to a name being introduced now. The sibling can be left alone; this one
cannot be renamed after a release. Suggest
`UNSUPPORTED_FEATURE.WRITE_ORDERING_WITH_UNKNOWN_COLUMN` with `"Invalid write
ordering: <cols> is not a column of the table."`, or a
non-`UNSUPPORTED_FEATURE` parent, since a missing column is not a feature gap.
##########
docs/sql-ref-syntax-ddl-create-table-datasource.md:
##########
@@ -98,6 +106,60 @@ as any order. For example, you can write COMMENT
table_comment after TBLPROPERTI
A list of key-value pairs that is used to tag the table definition.
+* **DISTRIBUTED BY PARTITION**
+
+ Requests that every write to the table be clustered by the table's
partitioning, so each
+ partition is written by a single task rather than by every task that holds
rows for it.
+
+ Requires the table to actually be partitioned, with `PARTITIONED BY` or
with
+ `CLUSTERED BY ... INTO ... BUCKETS`. Note that `CLUSTER BY` -- a different
clause from
+ `CLUSTERED BY ... INTO ... BUCKETS` -- does **not** qualify: it lists
clustering columns for the
+ data source to interpret rather than defining a partitioning, and it
cannot be combined with
+ `PARTITIONED BY` or `CLUSTERED BY ... INTO ... BUCKETS`, so a table using
it has no partitioning
+ to distribute by.
+
+* **ORDERED BY**
+
+ Requests a sort order for every write to the table, recorded on the table
so that later writes
+ honor it too. `UNORDERED` asks for no ordering at all, which is different
from omitting the
+ clause -- omitting it leaves the choice to the data source. The
parentheses are optional:
+ `ORDERED BY (a, b)` and `ORDERED BY a, b` are the same. The sort keys must
resolve against the
+ table's columns, so a `CREATE TABLE` with neither a column list nor `AS
SELECT` cannot use this
+ clause.
+
+ The distribution decides how far the order reaches, and this clause picks
one when
+ `DISTRIBUTED BY PARTITION` is absent: a bare `ORDERED BY` range-partitions
each write, so the
+ order holds across the whole table, while `LOCALLY ORDERED BY` asks for it
to hold within each
+ written file only, without a shuffle. `UNORDERED` on its own asks for no
distribution either.
+
+ When `DISTRIBUTED BY PARTITION` is given it decides the distribution
instead, and the order then
+ holds within each write task. `LOCALLY` therefore adds nothing beside it,
and `UNORDERED` beside
+ it contributes only "no sort keys":
+
+ ```sql
+ -- range-partition each write by id, so the order holds across the whole
table
+ CREATE TABLE t (id INT, c STRING) USING iceberg PARTITIONED BY (c) ORDERED
BY (id);
+
+ -- cluster each write by partition instead, and sort by id within each task
+ CREATE TABLE t (id INT, c STRING) USING iceberg PARTITIONED BY (c)
+ DISTRIBUTED BY PARTITION ORDERED BY (id);
+
+ -- cluster each write by partition, with no sort order; UNORDERED here is
the same as
+ -- omitting it, since DISTRIBUTED BY PARTITION already fixed the
distribution
+ CREATE TABLE t (id INT, c STRING) USING iceberg PARTITIONED BY (c)
+ DISTRIBUTED BY PARTITION UNORDERED;
+ ```
+
+ Both clauses are passed to the data source, which has to support them: a
data source that does
+ not advertise support for a write distribution and ordering rejects the
statement rather than
+ creating a table that silently lacks the requested layout. The built-in
data sources do not
+ support them.
+
+ What the data source records is a *default* for later writes, not a
statement about the data
+ already in the table: an individual write may override it, and rewriting
existing data to match
+ a newly requested layout is a separate operation. `SHOW CREATE TABLE`
reproduces the clauses and
Review Comment:
**Finding 5.** `SHOW CREATE TABLE` reproduces the clauses only for pairs the
syntax can spell, and emits nothing at all for the rest. That caveat is in the
PR description and in `ShowCreateTableExec`'s scaladoc, but not here, and this
page is what users read.
It is not a corner case. A connector that records a sort order without
touching its distribution mode reports `(null, non-empty)`, which has no clause
form. Measured on this branch, with a table reporting mode `null` and ordering
`id DESC NULLS LAST`:
CREATE TABLE n.t (
id INT)
USING foo
while `DESCRIBE TABLE EXTENDED` on the same table shows `Ordering` = `id
DESC NULLS LAST`. So that DDL runs and creates a table without the ordering.
One sentence covers it: `SHOW CREATE TABLE` reproduces the clauses when the
recorded pair has a clause form, and `DESCRIBE TABLE EXTENDED` reports both
values in every case.
##########
sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/ThriftServerWithSparkContextSuite.scala:
##########
@@ -214,7 +214,7 @@ trait ThriftServerWithSparkContextSuite extends
SharedThriftServer {
val sessionHandle = client.openSession(user, "")
val infoValue = client.getInfo(sessionHandle,
GetInfoType.CLI_ODBC_KEYWORDS)
// scalastyle:off line.size.limit
- assert(infoValue.getStringValue ==
"ADD,AFTER,AGGREGATE,ALIGN,ALL,ALTER,ALWAYS,ANALYZE,AND,ANTI,ANY,ANY_VALUE,APPLY,APPROX,ARCHIVE,ARRAY,AS,ASC,ASENSITIVE,ASOF,AT,ATOMIC,AUTHORIZATION,AUTO,BEGIN,BERNOULLI,BETWEEN,BIGINT,BIN,BINARY,BINDING,BIN_DISTRIBUTE_RATIO,BIN_END,BIN_START,BOOLEAN,BOTH,BUCKET,BUCKETS,BY,BYTE,CACHE,CALL,CALLED,CASCADE,CASE,CAST,CATALOG,CATALOGS,CDC,CHANGE,CHANGES,CHAR,CHARACTER,CHECK,CLEAR,CLOSE,CLUSTER,CLUSTERED,CODEGEN,COLLATE,COLLATION,COLLATIONS,COLLECTION,COLUMN,COLUMNS,COMMENT,COMMIT,COMPACT,COMPACTIONS,COMPENSATION,COMPUTE,CONCATENATE,CONDITION,CONSTRAINT,CONTAINS,CONTINUE,COST,CREATE,CROSS,CUBE,CURRENT,CURRENT_DATABASE,CURRENT_DATE,CURRENT_PATH,CURRENT_SCHEMA,CURRENT_TIME,CURRENT_TIMESTAMP,CURRENT_USER,CURSOR,DATA,DATABASE,DATABASES,DATE,DATEADD,DATEDIFF,DATE_ADD,DATE_DIFF,DAY,DAYOFYEAR,DAYS,DBPROPERTIES,DEC,DECIMAL,DECLARE,DEFAULT,DEFAULT_PATH,DEFINED,DEFINER,DELAY,DELETE,DELIMITED,DESC,DESCRIBE,DETERMINISTIC,DFS,DIRECTORIES,DIRECTORY,DISTANCE,DIST
INCT,DISTRIBUTE,DIV,DO,DOUBLE,DROP,ELSE,ELSEIF,EMPTY,END,ENFORCED,ERROR,ESCAPE,ESCAPED,EVOLUTION,EXACT,EXCEPT,EXCHANGE,EXCLUDE,EXCLUSIVE,EXECUTE,EXISTS,EXIT,EXPLAIN,EXPORT,EXTEND,EXTENDED,EXTERNAL,EXTRACT,FALSE,FETCH,FIELDS,FILEFORMAT,FILTER,FIRST,FLOAT,FLOW,FOLLOWING,FOR,FOREIGN,FORMAT,FORMATTED,FOUND,FROM,FULL,FUNCTION,FUNCTIONS,GENERATED,GEOGRAPHY,GEOMETRY,GLOBAL,GRANT,GROUP,GROUPING,HANDLER,HAVING,HISTORY,HOUR,HOURS,IDENTIFIED,IDENTIFIER,IDENTITY,IF,IGNORE,ILIKE,IMMEDIATE,IMPORT,IN,INCLUDE,INCLUSIVE,INCREMENT,INDEX,INDEXES,INNER,INPATH,INPUT,INPUTFORMAT,INSENSITIVE,INSERT,INT,INTEGER,INTERSECT,INTERVAL,INTO,INVOKER,IS,ITEMS,ITERATE,JOIN,JSON,JSON_EXISTS,JSON_TABLE,JSON_VALUE,KEY,KEYS,LANGUAGE,LAST,LATERAL,LAZY,LEADING,LEAVE,LEFT,LEVEL,LIKE,LIMIT,LINES,LIST,LOAD,LOCAL,LOCALTIME,LOCATION,LOCK,LOCKS,LOGICAL,LONG,LOOP,MACRO,MAP,MATCHED,MATCH_CONDITION,MATERIALIZED,MAX,MEASURE,MERGE,METRICS,MICROSECOND,MICROSECONDS,MILLISECOND,MILLISECONDS,MINUS,MINUTE,MINUTES,MODIFIES,MONTH,MONTHS,M
SCK,NAME,NAMESPACE,NAMESPACES,NANOSECOND,NANOSECONDS,NATURAL,NEAREST,NEXT,NO,NONE,NORELY,NOT,NULL,NULLS,NUMERIC,OF,OFFSET,ON,ONLY,OPEN,OPTION,OPTIONS,OR,ORDER,ORDINALITY,OUT,OUTER,OUTPUTFORMAT,OVER,OVERLAPS,OVERLAY,OVERWRITE,PARTITION,PARTITIONED,PARTITIONS,PATH,PERCENT,PIVOT,PLACING,POSITION,PRECEDING,PRIMARY,PRINCIPALS,PROCEDURE,PROCEDURES,PROPERTIES,PURGE,QUALIFY,QUARTER,QUERY,RANGE,READ,READS,REAL,RECORDREADER,RECORDWRITER,RECOVER,RECURSION,RECURSIVE,REDUCE,REFERENCES,REFRESH,RELY,RENAME,REPAIR,REPEAT,REPEATABLE,REPLACE,RESET,RESPECT,RESTRICT,RETURN,RETURNING,RETURNS,REVOKE,RIGHT,ROLE,ROLES,ROLLBACK,ROLLUP,ROW,ROWS,SCD,SCHEMA,SCHEMAS,SECOND,SECONDS,SECURITY,SELECT,SEMI,SEPARATED,SEQUENCE,SERDE,SERDEPROPERTIES,SESSION_USER,SET,SETS,SHORT,SHOW,SIMILARITY,SINGLE,SKEWED,SMALLINT,SOME,SORT,SORTED,SOURCE,SPECIFIC,SQL,SQLEXCEPTION,SQLSTATE,START,STATISTICS,STORED,STRATIFY,STREAM,STREAMING,STRING,STRUCT,SUBSTR,SUBSTRING,SYNC,SYSTEM,SYSTEM_PATH,SYSTEM_TIME,SYSTEM_VERSION,TABLE,TABLES,TAB
LESAMPLE,TARGET,TBLPROPERTIES,TERMINATED,THEN,TIME,TIMEDIFF,TIMESTAMP,TIMESTAMPADD,TIMESTAMPDIFF,TIMESTAMP_LTZ,TIMESTAMP_NTZ,TINYINT,TO,TOUCH,TRACK,TRAILING,TRANSACTION,TRANSACTIONS,TRANSFORM,TRIM,TRUE,TRUNCATE,TRY_CAST,TYPE,UNARCHIVE,UNBOUNDED,UNCACHE,UNIFORM,UNION,UNIQUE,UNKNOWN,UNLOCK,UNNEST,UNPIVOT,UNSET,UNTIL,UPDATE,USE,USER,USING,VALUE,VALUES,VAR,VARCHAR,VARIABLE,VARIANT,VERSION,VIEW,VIEWS,VOID,WATERMARK,WEEK,WEEKS,WHEN,WHERE,WHILE,WIDTH,WINDOW,WITH,WITHIN,WITHOUT,X,YEAR,YEARS,ZONE")
+ assert(infoValue.getStringValue ==
"ADD,AFTER,AGGREGATE,ALIGN,ALL,ALTER,ALWAYS,ANALYZE,AND,ANTI,ANY,ANY_VALUE,APPLY,APPROX,ARCHIVE,ARRAY,AS,ASC,ASENSITIVE,ASOF,AT,ATOMIC,AUTHORIZATION,AUTO,BEGIN,BERNOULLI,BETWEEN,BIGINT,BIN,BINARY,BINDING,BIN_DISTRIBUTE_RATIO,BIN_END,BIN_START,BOOLEAN,BOTH,BUCKET,BUCKETS,BY,BYTE,CACHE,CALL,CALLED,CASCADE,CASE,CAST,CATALOG,CATALOGS,CDC,CHANGE,CHANGES,CHAR,CHARACTER,CHECK,CLEAR,CLOSE,CLUSTER,CLUSTERED,CODEGEN,COLLATE,COLLATION,COLLATIONS,COLLECTION,COLUMN,COLUMNS,COMMENT,COMMIT,COMPACT,COMPACTIONS,COMPENSATION,COMPUTE,CONCATENATE,CONDITION,CONSTRAINT,CONTAINS,CONTINUE,COST,CREATE,CROSS,CUBE,CURRENT,CURRENT_DATABASE,CURRENT_DATE,CURRENT_PATH,CURRENT_SCHEMA,CURRENT_TIME,CURRENT_TIMESTAMP,CURRENT_USER,CURSOR,DATA,DATABASE,DATABASES,DATE,DATEADD,DATEDIFF,DATE_ADD,DATE_DIFF,DAY,DAYOFYEAR,DAYS,DBPROPERTIES,DEC,DECIMAL,DECLARE,DEFAULT,DEFAULT_PATH,DEFINED,DEFINER,DELAY,DELETE,DELIMITED,DESC,DESCRIBE,DETERMINISTIC,DFS,DIRECTORIES,DIRECTORY,DISTANCE,DIST
INCT,DISTRIBUTE,DISTRIBUTED,DIV,DO,DOUBLE,DROP,ELSE,ELSEIF,EMPTY,END,ENFORCED,ERROR,ESCAPE,ESCAPED,EVOLUTION,EXACT,EXCEPT,EXCHANGE,EXCLUDE,EXCLUSIVE,EXECUTE,EXISTS,EXIT,EXPLAIN,EXPORT,EXTEND,EXTENDED,EXTERNAL,EXTRACT,FALSE,FETCH,FIELDS,FILEFORMAT,FILTER,FIRST,FLOAT,FLOW,FOLLOWING,FOR,FOREIGN,FORMAT,FORMATTED,FOUND,FROM,FULL,FUNCTION,FUNCTIONS,GENERATED,GEOGRAPHY,GEOMETRY,GLOBAL,GRANT,GROUP,GROUPING,HANDLER,HAVING,HISTORY,HOUR,HOURS,IDENTIFIED,IDENTIFIER,IDENTITY,IF,IGNORE,ILIKE,IMMEDIATE,IMPORT,IN,INCLUDE,INCLUSIVE,INCREMENT,INDEX,INDEXES,INNER,INPATH,INPUT,INPUTFORMAT,INSENSITIVE,INSERT,INT,INTEGER,INTERSECT,INTERVAL,INTO,INVOKER,IS,ITEMS,ITERATE,JOIN,JSON,JSON_EXISTS,JSON_TABLE,JSON_VALUE,KEY,KEYS,LANGUAGE,LAST,LATERAL,LAZY,LEADING,LEAVE,LEFT,LEVEL,LIKE,LIMIT,LINES,LIST,LOAD,LOCAL,LOCALLY,LOCALTIME,LOCATION,LOCK,LOCKS,LOGICAL,LONG,LOOP,MACRO,MAP,MATCHED,MATCH_CONDITION,MATERIALIZED,MAX,MEASURE,MERGE,METRICS,MICROSECOND,MICROSECONDS,MILLISECOND,MILLISECONDS,MINUS,MINUTE,MINUTES,MOD
IFIES,MONTH,MONTHS,MSCK,NAME,NAMESPACE,NAMESPACES,NANOSECOND,NANOSECONDS,NATURAL,NEAREST,NEXT,NO,NONE,NORELY,NOT,NULL,NULLS,NUMERIC,OF,OFFSET,ON,ONLY,OPEN,OPTION,OPTIONS,OR,ORDER,ORDERED,ORDINALITY,OUT,OUTER,OUTPUTFORMAT,OVER,OVERLAPS,OVERLAY,OVERWRITE,PARTITION,PARTITIONED,PARTITIONS,PATH,PERCENT,PIVOT,PLACING,POSITION,PRECEDING,PRIMARY,PRINCIPALS,PROCEDURE,PROCEDURES,PROPERTIES,PURGE,QUALIFY,QUARTER,QUERY,RANGE,READ,READS,REAL,RECORDREADER,RECORDWRITER,RECOVER,RECURSION,RECURSIVE,REDUCE,REFERENCES,REFRESH,RELY,RENAME,REPAIR,REPEAT,REPEATABLE,REPLACE,RESET,RESPECT,RESTRICT,RETURN,RETURNING,RETURNS,REVOKE,RIGHT,ROLE,ROLES,ROLLBACK,ROLLUP,ROW,ROWS,SCD,SCHEMA,SCHEMAS,SECOND,SECONDS,SECURITY,SELECT,SEMI,SEPARATED,SEQUENCE,SERDE,SERDEPROPERTIES,SESSION_USER,SET,SETS,SHORT,SHOW,SIMILARITY,SINGLE,SKEWED,SMALLINT,SOME,SORT,SORTED,SOURCE,SPECIFIC,SQL,SQLEXCEPTION,SQLSTATE,START,STATISTICS,STORED,STRATIFY,STREAM,STREAMING,STRING,STRUCT,SUBSTR,SUBSTRING,SYNC,SYSTEM,SYSTEM_PATH,SYSTEM_TIME,SYS
TEM_VERSION,TABLE,TABLES,TABLESAMPLE,TARGET,TBLPROPERTIES,TERMINATED,THEN,TIME,TIMEDIFF,TIMESTAMP,TIMESTAMPADD,TIMESTAMPDIFF,TIMESTAMP_LTZ,TIMESTAMP_NTZ,TINYINT,TO,TOUCH,TRACK,TRAILING,TRANSACTION,TRANSACTIONS,TRANSFORM,TRIM,TRUE,TRUNCATE,TRY_CAST,TYPE,UNARCHIVE,UNBOUNDED,UNCACHE,UNIFORM,UNION,UNIQUE,UNKNOWN,UNLOCK,UNNEST,UNORDERED,UNPIVOT,UNSET,UNTIL,UPDATE,USE,USER,USING,VALUE,VALUES,VAR,VARCHAR,VARIABLE,VARIANT,VERSION,VIEW,VIEWS,VOID,WATERMARK,WEEK,WEEKS,WHEN,WHERE,WHILE,WIDTH,WINDOW,WITH,WITHIN,WITHOUT,X,YEAR,YEARS,ZONE")
Review Comment:
**Finding 1.** This list got the four new keywords, but its Spark Connect
JDBC sibling did not, and CI is red on it.
`SparkConnectDatabaseMetaDataSuite."SparkConnectDatabaseMetaData
getSQLKeywords"` asserts its own hardcoded list at
`sql/connect/client/jdbc/src/test/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaDataSuite.scala:213`.
It is the only failing test on `c6adea5c405` - one annotation on the "Report
test results" check run, and the failing "Build modules: ... connect ..." job
is the same test.
`getSQLKeywords` drops SQL:2003 reserved words, and all four new keywords
are non-reserved, so all four need adding:
- `...,DISTRIBUTE,DISTRIBUTED,DIV,...`
- `...,LOAD,LOCALLY,LOCATION,...`
- `...,OPTIONS,ORDERED,ORDINALITY,...`
- `...,UNLOCK,UNORDERED,UNPIVOT,...`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
