trino
Add support for default column values in engine
#25679
Open
Overview
Commits
9
Changes
View On GitHub

Add support for default column values in engine #25679

ebyhr wants to merge 9 commits into trinodb:master from ebyhr:ebi/engine-default-column-values
ebyhr
ebyhr28 days ago (edited 10 days ago)

Description

This PR adds support for the DEFAULT clause with literal values when creating a new table or adding a column.
The SHOW CREATE TABLE statement now includes the specified default values.
These defaults are respected in INSERT and MERGE operations.
Non-literal expressions such as <datetime value function>, USER, and similar are not supported in this PR.

<default clause> ::=
  DEFAULT <default option> 

<default option> ::=
    <literal> 
  | <datetime value function> 
  | USER
  | CURRENT_USER
  | CURRENT_ROLE
  | SESSION_USER
  | SYSTEM_USER
  | CURRENT_CATALOG
  | CURRENT_SCHEMA
  | CURRENT_PATH
  | <implicitly typed value specification>

Release notes

## General, Memory
* Add support for default column values. ({issue}`25679`)
ebyhr ebyhr added syntax-needs-review
cla-bot cla-bot added cla-signed
github-actions github-actions added iceberg
github-actions github-actions added delta-lake
github-actions github-actions added hive
github-actions github-actions added bigquery
github-actions github-actions added mongodb
github-actions github-actions added cassandra
github-actions github-actions added ignite
github-actions github-actions added memory
ebyhr ebyhr force pushed from 796b75e6 to 9b59d7c2 19 days ago
ebyhr ebyhr marked this pull request as ready for review 19 days ago
ebyhr ebyhr force pushed from 9b59d7c2 to 30ab353a 19 days ago
github-actions github-actions added docs
ebyhr ebyhr force pushed from 30ab353a to 61f6208e 19 days ago
ebyhr ebyhr force pushed from 61f6208e to 3eae1801 19 days ago
ebyhr ebyhr force pushed from 3eae1801 to 2e3b139b 19 days ago
ebyhr ebyhr requested a review from martint martint 19 days ago
ebyhr ebyhr force pushed from 2e3b139b to b905a014 19 days ago
ebyhr ebyhr force pushed from b905a014 to d75264c1 19 days ago
ebyhr ebyhr requested a review from Praveen2112 Praveen2112 13 days ago
ebyhr ebyhr requested a review from kasiafi kasiafi 13 days ago
kasiafi
kasiafi commented on 2025-05-12
Conversation is marked as resolved
Show resolved
core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java
185188 if (columns.containsKey(name.getValue().toLowerCase(ENGLISH))) {
186189 throw semanticException(DUPLICATE_COLUMN_NAME, column, "Column name '%s' specified more than once", name);
187190 }
191
 if (column.getDefaultValue().isPresent() && !plannerContext.getMetadata().getConnectorCapabilities(session, catalogHandle).contains(DEFAULT_COLUMN_VALUE)) {
kasiafi12 days ago

Could we check the connector capability once?

Conversation is marked as resolved
Show resolved
core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java
201208 columns.put(name.getValue().toLowerCase(ENGLISH), ColumnMetadata.builder()
202209 .setName(name.getValue().toLowerCase(ENGLISH))
203 .setType(getSupportedType(session, catalogHandle, properties, type))
210 .setType(supportedType)
211
 .setDefaultValue(column.getDefaultValue().map(value -> literalInterpreter.evaluate(value, type)))
kasiafi12 days ago

Please extract the call to literalInterpreter, catch exception and throw semanticException(INVALID_LITERAL)

kasiafi12 days ago

We must validate that given literal matches the declared type of the column. Not that LiteralInterpreter asserts the type only for generic literals.

Conversation is marked as resolved
Show resolved
core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java
152159 ColumnMetadata column = ColumnMetadata.builder()
153160 .setName(columnName.getValue())
154 .setType(getSupportedType(session, catalogHandle, tableMetadata.metadata().getProperties(), type))
161 .setType(supportedType)
162
 .setDefaultValue(element.getDefaultValue().map(value -> literalInterpreter.evaluate(value, type)))
kasiafi12 days ago

See comment in CreateTableTask.

Conversation is marked as resolved
Show resolved
core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java
kasiafi12 days ago

Can default value be null? If so, should we check it here against the non-null constraint, or only fail if the default value is actually used? What does the spec say?

ebyhr12 days ago (edited 12 days ago)

The spec allows null as the default value. I intentionally avoided supporting null to minimize the scope of this PR.

<default option> ::=
  ...
  | <implicitly typed value specification> 

<implicitly typed value specification> 

<implicitly typed value specification> ::=
    <null specification> 
  | <empty specification> 

<null specification> ::=
  NULL
kasiafi12 days ago

If null is allowed by the spec, then I think we should support it right away. Otherwise we might create inconsistent user experience.

Conversation is marked as resolved
Show resolved
core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java
kasiafi12 days ago

Same question about null default value vs non-null constraint.

Conversation is marked as resolved
Show resolved
core/trino-main/src/main/java/io/trino/sql/planner/QueryPlanner.java
kasiafi12 days ago (edited 12 days ago)

I'm not sure of the logic here. @djsstarburst could you please help review the changes related to merge?

djsstarburst12 days ago

This looks okay to me assumjng thgat defaultColumnValues cannot contain null if the column is in nonNullableColumnHandles.

ebyhr12 days ago (edited 12 days ago)👍 1

defaultColumnValues always don't contain null regardless of the nullability at this time.

ebyhr ebyhr force pushed from d75264c1 to 995de6e6 12 days ago
ebyhr
kasiafi
kasiafi commented on 2025-05-13
Praveen2112
Praveen2112 commented on 2025-05-13
ebyhr ebyhr force pushed from 995de6e6 to f18ad963 11 days ago
ebyhr Extract literals in SqlBase.g4
3c00cc3a
ebyhr Add support for default column values in parser
9351cb13
ebyhr Add support for default column values in engine
b53119e0
ebyhr ebyhr force pushed from f18ad963 to cecbfec6 11 days ago
ebyhr Add connector test for default column values
62525cac
ebyhr ebyhr force pushed from cecbfec6 to 62525cac 11 days ago
ebyhr Support default column values in Memory connector
c0ebad12
ebyhr
kasiafi
kasiafi commented on 2025-05-15
ebyhr fixup! Add support for default column values in parser
b72c8e60
ebyhr ebyhr force pushed from 6618e8b3 to 29b8e3b5 9 days ago
ebyhr ebyhr force pushed from 29b8e3b5 to ccfec5d4 5 days ago
ebyhr
ebyhr fixup! Add support for default column values in engine
ac679884
ebyhr fixup! Add connector test for default column values
e7d9bdd5
ebyhr ebyhr force pushed from ccfec5d4 to e7d9bdd5 5 days ago
ebyhr fixup! Add support for default column values in engine
0b2730f7
ebyhr ebyhr requested a review from kasiafi kasiafi 2 days ago
ebyhr ebyhr requested a review from Praveen2112 Praveen2112 2 days ago

Login to write a write a comment.

Login via GitHub

Reviewers
Assignees
No one assigned
Labels
cla-signed syntax-needs-review docs iceberg delta-lake hive bigquery mongodb cassandra ignite memory
Milestone
No milestone
Terms Of Use Privacy Policy Refund Policy Impressum