Skip to content
Prev Previous commit
Next Next commit
update
  • Loading branch information
panbingkun committed Jul 2, 2024
commit 5733fbbfc225b4795a0220cf23d6bc9d5fe79fe1
20 changes: 15 additions & 5 deletions docs/sql-ref-syntax-ddl-alter-table.md
Original file line number Diff line number Diff line change
Expand Up @@ -236,20 +236,30 @@ ALTER TABLE table_identifier DROP [ IF EXISTS ] partition_spec [PURGE]

### SET AND UNSET

#### SET TABLE PROPERTIES
#### SET PROPERTIES

`ALTER TABLE SET` command is used for setting the table properties. If a particular property was already set,
this overrides the old value with the new one.

`ALTER TABLE UNSET` is used to drop the table property.

##### Syntax

```sql
-- Set Table Properties
-- Set Properties
ALTER TABLE table_identifier SET TBLPROPERTIES ( key1 = val1, key2 = val2, ... )
```

#### UNSET PROPERTIES

`ALTER TABLE UNSET` command is used to drop the table property.

-- Unset Table Properties
**Note:** If the specified property key does not exist, when you use the v1 command and do not specify `IF EXISTS`,
it will throw the error-condition `UNSET_NONEXISTENT_PROPERTIES` and finally `failed`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we change the v1 behavior as well? It's not a breaking change to change from failure to no failure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we should avoid mentioning v1/v2 in the doc. It's an internal concept and should not be exposed to end users

Copy link
Contributor Author

@panbingkun panbingkun Jul 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we change the v1 behavior as well? It's not a breaking change to change from failure to no failure.

Sure, let me update it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we should avoid mentioning v1/v2 in the doc. It's an internal concept and should not be exposed to end users

Okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we change the v1 behavior as well? It's not a breaking change to change from failure to no failure.

Done.

however regardless of whether `IF EXISTS` is set or not, the v2 command will ignore it and finally `succeed`.

##### Syntax

```sql
-- Unset Properties
ALTER TABLE table_identifier UNSET TBLPROPERTIES [ IF EXISTS ] ( key1, key2, ... )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's remove [ IF EXISTS ] as a way to deprecate. It's no longer effective.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ trait AlterTableUnsetTblPropertiesSuiteBase extends command.AlterTableUnsetTblPr
getTableProperties(tableIdent).getOrElse(key, null)
}

/**
* When using the v1 command to unset `non-existent` properties, the command will
* throw the error-condition `"UNSET_NONEXISTENT_PROPERTIES` and finally `failed`
*/
test("alter table unset non-existent properties") {
withNamespaceAndTable("ns", "tbl") { t =>
sql(s"CREATE TABLE $t (col1 int, col2 string, a int, b int) $defaultUsing")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ class AlterTableUnsetTblPropertiesSuite
getTableMetadata(tableIdent).properties.asScala.toMap.getOrElse(key, null)
}

/**
* When using the v2 command to unset `non-existent` properties,
* the command will ignore `non-existent` properties and finally succeed
*/
test("alter table unset non-existent properties") {
withNamespaceAndTable("ns", "tbl") { t =>
sql(s"CREATE TABLE $t (col1 int, col2 string, a int, b int) $defaultUsing")
Expand All @@ -60,8 +64,6 @@ class AlterTableUnsetTblPropertiesSuite
sql(s"ALTER TABLE $t SET TBLPROPERTIES ('k1' = 'v1', 'k2' = 'v2', 'k3' = 'v3')")
checkTblProps(tableIdent, Map("k1" -> "v1", "k2" -> "v2", "k3" -> "v3"))

// when using the v2 command, when unsetting `non-existent` properties,
// The command will ignore `non-existent` properties and finally succeed
// property to unset does not exist
sql(s"ALTER TABLE $t UNSET TBLPROPERTIES ('k3', 'k4')")
checkTblProps(tableIdent, Map("k1" -> "v1", "k2" -> "v2"))
Expand Down