Skip to content

Commit

Permalink
[apache#1392] feat(api): add defaultValue() in Column API (apache#1393
Browse files Browse the repository at this point in the history
)

### What changes were proposed in this pull request?

- add `defaultValue()` in Column API
- remove the duplicate methods from the subclass.

### Why are the changes needed?

Fix: apache#1392 

### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

There is no specific implementation, so testing is not required.
  • Loading branch information
mchades authored Jan 15, 2024
1 parent ce1d1b8 commit 3a770cc
Show file tree
Hide file tree
Showing 13 changed files with 106 additions and 48 deletions.
8 changes: 7 additions & 1 deletion api/src/main/java/com/datastrato/gravitino/rel/Column.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package com.datastrato.gravitino.rel;

import com.datastrato.gravitino.NameIdentifier;
import com.datastrato.gravitino.rel.expressions.Expression;
import com.datastrato.gravitino.rel.types.Type;
import java.util.Map;

Expand All @@ -18,6 +19,8 @@
*/
public interface Column {

Expression DEFAULT_VALUE_NOT_SET = () -> Expression.EMPTY_EXPRESSION;

/** @return The name of this column. */
String name();

Expand All @@ -33,5 +36,8 @@ public interface Column {
/** @return True if this column is an auto-increment column. Default is false. */
boolean autoIncrement();

// TODO. Support column default value. @Jerry
/**
* @return The default value of this column, {@link Column#DEFAULT_VALUE_NOT_SET} if not specified
*/
Expression defaultValue();
}
10 changes: 10 additions & 0 deletions api/src/test/java/com/datastrato/gravitino/rel/TestTransforms.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ public boolean nullable() {
public boolean autoIncrement() {
return false;
}

@Override
public Expression defaultValue() {
return Column.DEFAULT_VALUE_NOT_SET;
}
};
String[] fieldName = new String[] {column.name()};

Expand Down Expand Up @@ -107,6 +112,11 @@ public boolean nullable() {
public boolean autoIncrement() {
return false;
}

@Override
public Expression defaultValue() {
return Column.DEFAULT_VALUE_NOT_SET;
}
};
// partition by foo(col_1, 'bar')
NamedReference.FieldReference arg1 = field(column.name());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,20 @@

/** Represents a column in the Jdbc column. */
public class JdbcColumn extends BaseColumn {
private String defaultValue;

private List<String> properties;

private JdbcColumn() {}

public String getDefaultValue() {
return defaultValue;
}

public List<String> getProperties() {
return properties;
}

/** A builder class for constructing JdbcColumn instances. */
public static class Builder extends BaseColumnBuilder<Builder, JdbcColumn> {

/**
* The default value for this field. This value will be used if the corresponding value is null.
*/
private String defaultValue;

/** Attribute value of the field, such as AUTO_INCREMENT, PRIMARY KEY, etc. */
private List<String> properties;

public Builder withDefaultValue(String defaultValue) {
this.defaultValue = defaultValue;
return this;
}

public Builder withProperties(List<String> properties) {
this.properties = properties;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ protected JdbcColumn extractJdbcColumnFromResultSet(ResultSet resultSet) throws
.withComment(null)
.withType(typeConverter.toGravitinoType(resultSet.getString("TYPE_NAME")))
.withNullable(resultSet.getBoolean("NULLABLE"))
.withDefaultValue(resultSet.getString("COLUMN_DEF"))
// TODO: uncomment this once we support column default values.
// .withDefaultValue(resultSet.getString("COLUMN_DEF"))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,9 @@ public void testOperationTable() {
Assertions.assertEquals(jdbcColumn.comment(), column.comment());
Assertions.assertEquals(jdbcColumn.dataType(), column.dataType());
Assertions.assertEquals(jdbcColumn.nullable(), column.nullable());
Assertions.assertEquals(
jdbcColumn.getDefaultValue(), ((JdbcColumn) column).getDefaultValue());
// TODO: uncomment this once we support column default values.
// Assertions.assertEquals(
// jdbcColumn.getDefaultValue(), ((JdbcColumn) column).getDefaultValue());
}

String newName = "table2";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ public JdbcTable load(String databaseName, String tableName) throws NoSuchTableE
.withType(typeConverter.toGravitinoType(columnDefinition.getColDataType()))
.withNullable(nullable)
.withComment(comment)
.withDefaultValue("NULL".equals(defaultValue) ? null : defaultValue)
// TODO: uncomment this once we support column default values.
// .withDefaultValue("NULL".equals(defaultValue) ? null : defaultValue)
.withProperties(properties)
.build());
}
Expand Down Expand Up @@ -133,7 +134,8 @@ private JdbcColumn getJdbcColumnFromCreateTable(CreateTable createTable, String
.withType(typeConverter.toGravitinoType(columnDefinition.getColDataType()))
.withNullable(nullable)
.withComment(comment)
.withDefaultValue("NULL".equals(defaultValue) ? null : defaultValue)
// TODO: uncomment this once we support column default values.
// .withDefaultValue("NULL".equals(defaultValue) ? null : defaultValue)
.withProperties(properties)
.build();
}
Expand Down Expand Up @@ -378,7 +380,8 @@ private String updateColumnNullabilityDefinition(
JdbcColumn updateColumn =
new JdbcColumn.Builder()
.withName(col)
.withDefaultValue(column.getDefaultValue())
// TODO: uncomment this once we support column default values.
// .withDefaultValue(column.getDefaultValue())
.withNullable(change.nullable())
.withProperties(column.getProperties())
.withType(column.dataType())
Expand Down Expand Up @@ -416,7 +419,8 @@ private String updateColumnCommentFieldDefinition(
JdbcColumn updateColumn =
new JdbcColumn.Builder()
.withName(col)
.withDefaultValue(column.getDefaultValue())
// TODO: uncomment this once we support column default values.
// .withDefaultValue(column.getDefaultValue())
.withNullable(column.nullable())
.withProperties(column.getProperties())
.withType(column.dataType())
Expand Down Expand Up @@ -471,7 +475,8 @@ private String renameColumnFieldDefinition(
.withType(column.dataType())
.withComment(column.comment())
.withProperties(column.getProperties())
.withDefaultValue(column.getDefaultValue())
// TODO: uncomment this once we support column default values.
// .withDefaultValue(column.getDefaultValue())
.withNullable(column.nullable())
.build();
return appendColumnDefinition(newColumn, sqlBuilder).toString();
Expand Down Expand Up @@ -561,9 +566,10 @@ private StringBuilder appendColumnDefinition(JdbcColumn column, StringBuilder sq
sqlBuilder.append("NOT NULL ");
}
// Add DEFAULT value if specified
if (StringUtils.isNotEmpty(column.getDefaultValue())) {
sqlBuilder.append("DEFAULT '").append(column.getDefaultValue()).append("'").append(SPACE);
}
// TODO: uncomment this once we support column default values.
// if (StringUtils.isNotEmpty(column.getDefaultValue())) {
// sqlBuilder.append("DEFAULT '").append(column.getDefaultValue()).append("'").append(SPACE);
// }

// Add column properties if specified
if (CollectionUtils.isNotEmpty(column.getProperties())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ private List<JdbcColumn> selectColumnInfoAndExecute(
JdbcColumn.Builder builder =
new JdbcColumn.Builder()
.withName(columnName)
.withDefaultValue(extractDefaultValue(resultSet.getString("column_default")))
// TODO: uncomment this once we support column default values.
// .withDefaultValue(extractDefaultValue(resultSet.getString("column_default")))
.withNullable("YES".equalsIgnoreCase(resultSet.getString("is_nullable")))
.withType(typeConverter.toGravitinoType(colDataType))
.withAutoIncrement("YES".equalsIgnoreCase(resultSet.getString("is_identity")));
Expand Down Expand Up @@ -304,9 +305,10 @@ private StringBuilder appendColumnDefinition(JdbcColumn column, StringBuilder sq
sqlBuilder.append("NOT NULL ");
}
// Add DEFAULT value if specified
if (StringUtils.isNotEmpty(column.getDefaultValue())) {
sqlBuilder.append("DEFAULT '").append(column.getDefaultValue()).append("'").append(SPACE);
}
// TODO: uncomment this once we support column default values.
// if (StringUtils.isNotEmpty(column.getDefaultValue())) {
// sqlBuilder.append("DEFAULT '").append(column.getDefaultValue()).append("'").append(SPACE);
// }

// Add column properties if specified
if (CollectionUtils.isNotEmpty(column.getProperties())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import com.datastrato.gravitino.json.JsonUtils;
import com.datastrato.gravitino.rel.Column;
import com.datastrato.gravitino.rel.expressions.Expression;
import com.datastrato.gravitino.rel.types.Type;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
Expand Down Expand Up @@ -81,6 +82,11 @@ public boolean autoIncrement() {
return autoIncrement;
}

@Override
public Expression defaultValue() {
throw new UnsupportedOperationException("Column default value is not supported yet.");
}

/**
* Creates a new Builder to build a Column DTO.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package com.datastrato.gravitino.catalog.rel;

import com.datastrato.gravitino.rel.Column;
import com.datastrato.gravitino.rel.expressions.Expression;
import com.datastrato.gravitino.rel.types.Type;
import javax.annotation.Nullable;
import lombok.EqualsAndHashCode;
Expand All @@ -26,6 +27,8 @@ public abstract class BaseColumn implements Column {

protected boolean autoIncrement;

protected Expression defaultValue;

/**
* Returns the name of the column.
*
Expand Down Expand Up @@ -69,6 +72,14 @@ public boolean autoIncrement() {
return autoIncrement;
}

/**
* @return The default value of this column, {@link Column#DEFAULT_VALUE_NOT_SET} if not specified
*/
@Override
public Expression defaultValue() {
return defaultValue;
}

/**
* Builder interface for creating instances of {@link BaseColumn}.
*
Expand All @@ -86,6 +97,8 @@ interface Builder<SELF extends BaseColumn.Builder<SELF, T>, T extends BaseColumn

SELF withAutoIncrement(boolean autoIncrement);

SELF withDefaultValue(Expression defaultValue);

T build();
}

Expand All @@ -103,6 +116,7 @@ public abstract static class BaseColumnBuilder<
protected Type dataType;
protected boolean nullable = true;
protected boolean autoIncrement = false;
protected Expression defaultValue;

/**
* Sets the name of the column.
Expand Down Expand Up @@ -164,6 +178,18 @@ public SELF withAutoIncrement(boolean autoIncrement) {
return self();
}

/**
* Sets the default value of the column.
*
* @param defaultValue The default value of the column.
* @return The builder instance.
*/
@Override
public SELF withDefaultValue(Expression defaultValue) {
this.defaultValue = defaultValue;
return self();
}

/**
* Builds the instance of the column with the provided attributes.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ protected TestColumn internalBuild() {
column.comment = comment;
column.dataType = dataType;
column.nullable = nullable;
column.defaultValue = defaultValue;

return column;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ protected static void assertionsTableInfo(
Assertions.assertEquals(columns.get(i).nullable(), table.columns()[i].nullable());
Assertions.assertEquals(columns.get(i).comment(), table.columns()[i].comment());
Assertions.assertEquals(columns.get(i).autoIncrement(), table.columns()[i].autoIncrement());
Assertions.assertEquals(
columns.get(i).getDefaultValue(), ((JdbcColumn) table.columns()[i]).getDefaultValue());
// TODO: uncomment this after default value is supported.
// Assertions.assertEquals(
// columns.get(i).getDefaultValue(), ((JdbcColumn) table.columns()[i]).getDefaultValue());
if (null != columns.get(i).getProperties()) {
Assertions.assertEquals(
columns.get(i).getProperties(), ((JdbcColumn) table.columns()[i]).getProperties());
Expand Down
Loading

0 comments on commit 3a770cc

Please sign in to comment.