From 2419cbe8d0adc14414691aa1e554b553b6e10d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Paksy?= Date: Tue, 7 Jul 2026 15:32:33 +0200 Subject: [PATCH 1/2] PHOENIX-7955 CREATE TABLE IF NOT EXISTS should not mutate existing HBase table properties Problem: When CREATE TABLE IF NOT EXISTS was executed against an already-existing table, Phoenix would overwrite the HBase-level table and column family properties (e.g. REGION_REPLICATION, VERSIONS, TTL) with the values from the new DDL statement. This is unexpected - IF NOT EXISTS should be a no-op if the table already exists. Fix: In ConnectionQueryServicesImpl changed ensureTableCreated call for TABLE types - passes modifyExistingMetaData = false when the table type is TABLE, so existing properties are not overwritten. Testing: Added new test in CreateTableIT which reproduces the problem and verifies the fix. Co-Authored-By: Claude Opus 4.6 --- .../query/ConnectionQueryServicesImpl.java | 3 +- .../apache/phoenix/end2end/CreateTableIT.java | 58 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java index 2c27a57fd37..7f6c1c5ea29 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java @@ -2550,8 +2550,9 @@ public MetaDataMutationResult createTable(final List tableMetaData, ) { // For views this will ensure that metadata already exists // For tables and indexes, this will create the metadata if it doesn't already exist + boolean modifyExistingMetaData = tableType != PTableType.TABLE; ensureTableCreated(physicalTableNameBytes, null, tableType, tableProps, families, splits, - true, isNamespaceMapped, isDoNotUpgradePropSet); + modifyExistingMetaData, isNamespaceMapped, isDoNotUpgradePropSet); } ImmutableBytesWritable ptr = new ImmutableBytesWritable(); if (tableType == PTableType.INDEX) { // Index on view diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java index f4f57bafc8a..42629cf4917 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java @@ -25,6 +25,7 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -1769,4 +1770,61 @@ private int checkGuidePostWidth(String tableName) throws Exception { } } + @Test + public void testCreateTableDoesNotMutateExistingHBaseTableProperties() throws Exception { + String tableName = generateUniqueName(); + Properties props = new Properties(); + + // Step 1: Create table with VERSIONS=1 and TTL=86400 + String createDdl = "CREATE TABLE " + tableName + " (" + " PK VARCHAR NOT NULL PRIMARY KEY," + + " COL1 VARCHAR," + " COL2 INTEGER" + " ) VERSIONS=1, TTL=86400"; + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { + conn.createStatement().execute(createDdl); + } + + // Verify initial HBase table properties + Admin admin = driver.getConnectionQueryServices(getUrl(), props).getAdmin(); + TableDescriptor descBefore = admin.getDescriptor(TableName.valueOf(tableName)); + ColumnFamilyDescriptor[] cfsBefore = descBefore.getColumnFamilies(); + assertEquals(1, cfsBefore.length); + assertEquals(1, cfsBefore[0].getMaxVersions()); + assertEquals(86400, cfsBefore[0].getTimeToLive()); + + // Step 2: Execute CREATE TABLE IF NOT EXISTS with different properties + String createIfNotExistsDdl = + "CREATE TABLE IF NOT EXISTS " + tableName + " (" + " PK VARCHAR NOT NULL PRIMARY KEY," + + " COL1 VARCHAR," + " COL2 INTEGER" + " ) VERSIONS=5, TTL=120000"; + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { + conn.createStatement().execute(createIfNotExistsDdl); + } + + // Step 3: Verify that HBase table properties are NOT changed + TableDescriptor descAfter = admin.getDescriptor(TableName.valueOf(tableName)); + ColumnFamilyDescriptor[] cfsAfter = descAfter.getColumnFamilies(); + assertEquals(1, cfsAfter.length); + assertEquals("VERSIONS should not be modified by IF NOT EXISTS", 1, + cfsAfter[0].getMaxVersions()); + assertEquals("TTL should not be modified by IF NOT EXISTS", 86400, cfsAfter[0].getTimeToLive()); + + // Step 4: Execute CREATE TABLE without IF NOT EXISTS with different properties + String createWithoutIfNotExistsDdl = + "CREATE TABLE " + tableName + " (" + " PK VARCHAR NOT NULL PRIMARY KEY," + " COL1 VARCHAR," + + " COL2 INTEGER" + " ) VERSIONS=6, TTL=120000"; + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { + // Should fail as table exists already. + TableAlreadyExistsException exception = assertThrows(TableAlreadyExistsException.class, + () -> conn.createStatement().execute(createWithoutIfNotExistsDdl)); + assertEquals("ERROR 1013 (42M04): Table already exists. tableName=N000063", + exception.getMessage()); + } + + // Step 5: Verify that HBase table properties are still NOT changed + TableDescriptor descAfter2 = admin.getDescriptor(TableName.valueOf(tableName)); + ColumnFamilyDescriptor[] cfsAfter2 = descAfter2.getColumnFamilies(); + assertEquals(1, cfsAfter2.length); + assertEquals("VERSIONS should not be modified for existing table", 1, + cfsAfter2[0].getMaxVersions()); + assertEquals("TTL should not be modified for existing table", 86400, + cfsAfter2[0].getTimeToLive()); + } } From 7781f282439c8438f5edbacccff19aff049f22e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Paksy?= Date: Fri, 10 Jul 2026 16:17:24 +0200 Subject: [PATCH 2/2] PHOENIX-7955: Implement different approach --- .../query/ConnectionQueryServices.java | 2 +- .../query/ConnectionQueryServicesImpl.java | 32 ++++++------- .../ConnectionlessQueryServicesImpl.java | 2 +- .../DelegateConnectionQueryServices.java | 4 +- .../apache/phoenix/schema/MetaDataClient.java | 2 +- .../phoenix/end2end/AppendOnlySchemaIT.java | 2 +- .../apache/phoenix/end2end/CreateTableIT.java | 45 ++++++++++++++----- 7 files changed, 56 insertions(+), 33 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java index 8808d67d764..c45d7d5d3c2 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java @@ -163,7 +163,7 @@ public MetaDataMutationResult getFunctions(PName tenantId, public MetaDataMutationResult createTable(List tableMetaData, byte[] tableName, PTableType tableType, Map tableProps, List>> families, byte[][] splits, boolean isNamespaceMapped, - boolean allocateIndexId, boolean isDoNotUpgradePropSet, PTable parentTable) throws SQLException; + boolean allocateIndexId, boolean isDoNotUpgradePropSet, PTable parentTable, boolean createIfNotExists) throws SQLException; public MetaDataMutationResult dropTable(List tableMetadata, PTableType tableType, boolean cascade) throws SQLException; diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java index 7f6c1c5ea29..553adf658bc 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java @@ -1806,7 +1806,8 @@ boolean ensureNamespaceCreated(String schemaName) throws SQLException { private TableDescriptor ensureTableCreated(byte[] physicalTableName, byte[] parentPhysicalTableName, PTableType tableType, Map props, List>> families, byte[][] splits, - boolean modifyExistingMetaData, boolean isNamespaceMapped, boolean isDoNotUpgradePropSet) + boolean modifyExistingMetaData, boolean isNamespaceMapped, boolean isDoNotUpgradePropSet, + boolean createIfNotExists) throws SQLException { SQLException sqlE = null; TableDescriptor existingDesc = null; @@ -2052,8 +2053,8 @@ private TableDescriptor ensureTableCreated(byte[] physicalTableName, return null; // Indicate that no metadata was changed } - // Do not call modifyTable for SYSTEM tables - if (tableType != PTableType.SYSTEM) { + // Do not call modifyTable for: SYSTEM tables or when CREATE IF NOT EXISTS was used. + if (tableType != PTableType.SYSTEM && !createIfNotExists) { modifyTable(physicalTableName, newDesc.build(), true); } return result; @@ -2423,12 +2424,13 @@ private MetaDataMutationResult metaDataCoprocessorExec(String tableName, byte[] private void ensureViewIndexTableCreated(byte[] physicalTableName, byte[] parentPhysicalTableName, Map tableProps, List>> families, - byte[][] splits, long timestamp, boolean isNamespaceMapped) throws SQLException { + byte[][] splits, long timestamp, boolean isNamespaceMapped, boolean createIfNotExists) throws SQLException { byte[] physicalIndexName = MetaDataUtil.getViewIndexPhysicalName(physicalTableName); tableProps.put(MetaDataUtil.IS_VIEW_INDEX_TABLE_PROP_NAME, TRUE_BYTES_AS_STRING); TableDescriptor desc = ensureTableCreated(physicalIndexName, parentPhysicalTableName, - PTableType.TABLE, tableProps, families, splits, true, isNamespaceMapped, false); + PTableType.TABLE, tableProps, families, splits, true, isNamespaceMapped, false, + createIfNotExists); if (desc != null) { if ( !Boolean.TRUE.equals( @@ -2522,7 +2524,7 @@ public MetaDataMutationResult createTable(final List tableMetaData, final byte[] physicalTableName, PTableType tableType, Map tableProps, final List>> families, byte[][] splits, boolean isNamespaceMapped, final boolean allocateIndexId, final boolean isDoNotUpgradePropSet, - final PTable parentTable) throws SQLException { + final PTable parentTable, boolean createIfNotExists) throws SQLException { List childLinkMutations = MetaDataUtil.removeChildLinkMutations(tableMetaData); byte[][] rowKeyMetadata = new byte[3][]; Mutation m = MetaDataUtil.getPutOnlyTableHeaderRow(tableMetaData); @@ -2550,9 +2552,8 @@ public MetaDataMutationResult createTable(final List tableMetaData, ) { // For views this will ensure that metadata already exists // For tables and indexes, this will create the metadata if it doesn't already exist - boolean modifyExistingMetaData = tableType != PTableType.TABLE; ensureTableCreated(physicalTableNameBytes, null, tableType, tableProps, families, splits, - modifyExistingMetaData, isNamespaceMapped, isDoNotUpgradePropSet); + true, isNamespaceMapped, isDoNotUpgradePropSet, createIfNotExists); } ImmutableBytesWritable ptr = new ImmutableBytesWritable(); if (tableType == PTableType.INDEX) { // Index on view @@ -2563,7 +2564,7 @@ public MetaDataMutationResult createTable(final List tableMetaData, // For view index, the physical table name is _IDX_+ logical table name format ensureViewIndexTableCreated( tenantIdBytes.length == 0 ? null : PNameFactory.newName(tenantIdBytes), - physicalTableName, MetaDataUtil.getClientTimeStamp(m), isNamespaceMapped); + physicalTableName, MetaDataUtil.getClientTimeStamp(m), isNamespaceMapped, createIfNotExists); } } } else if (tableType == PTableType.TABLE && MetaDataUtil.isMultiTenant(m, kvBuilder, ptr)) { // Create @@ -2598,7 +2599,7 @@ public MetaDataMutationResult createTable(final List tableMetaData, } ensureViewIndexTableCreated(physicalTableNameBytes, physicalTableNameBytes, tableProps, familiesPlusDefault, MetaDataUtil.isSalted(m, kvBuilder, ptr) ? splits : null, - MetaDataUtil.getClientTimeStamp(m), isNamespaceMapped); + MetaDataUtil.getClientTimeStamp(m), isNamespaceMapped, createIfNotExists); } // Avoid the client-server RPC if this is not a view creation @@ -2883,13 +2884,13 @@ private static Map createPropertiesMap(Map htableP } private void ensureViewIndexTableCreated(PName tenantId, byte[] physicalIndexTableName, - long timestamp, boolean isNamespaceMapped) throws SQLException { + long timestamp, boolean isNamespaceMapped, boolean createIfNotExists) throws SQLException { String name = Bytes .toString(SchemaUtil.getParentTableNameFromIndexTable(physicalIndexTableName, MetaDataUtil.VIEW_INDEX_TABLE_PREFIX)) .replace(QueryConstants.NAMESPACE_SEPARATOR, QueryConstants.NAME_SEPARATOR); PTable table = getTable(tenantId, name, timestamp); - ensureViewIndexTableCreated(table, timestamp, isNamespaceMapped); + ensureViewIndexTableCreated(table, timestamp, isNamespaceMapped, createIfNotExists); } private PTable getTable(PName tenantId, String fullTableName, long timestamp) @@ -2918,7 +2919,8 @@ private PTable getTable(PName tenantId, String fullTableName, long timestamp) return table; } - private void ensureViewIndexTableCreated(PTable table, long timestamp, boolean isNamespaceMapped) + private void ensureViewIndexTableCreated(PTable table, long timestamp, boolean isNamespaceMapped, + boolean createIfNotExists) throws SQLException { byte[] physicalTableName = table.getPhysicalName().getBytes(); TableDescriptor htableDesc = this.getTableDescriptor(physicalTableName); @@ -2955,7 +2957,7 @@ private void ensureViewIndexTableCreated(PTable table, long timestamp, boolean i byte[] viewPhysicalTableName = MetaDataUtil .getNamespaceMappedName(table.getName(), isNamespaceMapped).getBytes(StandardCharsets.UTF_8); ensureViewIndexTableCreated(viewPhysicalTableName, physicalTableName, tableProps, families, - splits, timestamp, isNamespaceMapped); + splits, timestamp, isNamespaceMapped, createIfNotExists); } @Override @@ -3093,7 +3095,7 @@ public MetaDataResponse call(MetaDataService instance) throws IOException { Boolean.TRUE .equals(PBoolean.INSTANCE.toObject(ptr.get(), ptr.getOffset(), ptr.getLength())) ) { - this.ensureViewIndexTableCreated(table, timestamp, table.isNamespaceMapped()); + this.ensureViewIndexTableCreated(table, timestamp, table.isNamespaceMapped(), true); } else { this.ensureViewIndexTableDropped(table.getPhysicalName().getBytes(), timestamp); } diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java index abbc08ac31a..aac12bf83e7 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java @@ -363,7 +363,7 @@ private static List generateRegionLocations(byte[] physicalName public MetaDataMutationResult createTable(List tableMetaData, byte[] physicalName, PTableType tableType, Map tableProps, List>> families, byte[][] splits, boolean isNamespaceMapped, - boolean allocateIndexId, boolean isDoNotUpgradePropSet, PTable parentTable) + boolean allocateIndexId, boolean isDoNotUpgradePropSet, PTable parentTable, boolean createIfNotExists) throws SQLException { if ( tableType == PTableType.INDEX diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java b/phoenix-core-client/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java index 24a42297091..610c4eefa25 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java @@ -164,10 +164,10 @@ public MetaDataMutationResult getTable(PName tenantId, byte[] schemaBytes, byte[ public MetaDataMutationResult createTable(List tableMetaData, byte[] physicalName, PTableType tableType, Map tableProps, List>> families, byte[][] splits, boolean isNamespaceMapped, - boolean allocateIndexId, boolean isDoNotUpgradePropSet, PTable parentTable) + boolean allocateIndexId, boolean isDoNotUpgradePropSet, PTable parentTable, boolean createIfNotExists) throws SQLException { return getDelegate().createTable(tableMetaData, physicalName, tableType, tableProps, families, - splits, isNamespaceMapped, allocateIndexId, isDoNotUpgradePropSet, parentTable); + splits, isNamespaceMapped, allocateIndexId, isDoNotUpgradePropSet, parentTable, createIfNotExists); } @Override diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java b/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java index 22833e24945..e1221bf5a9e 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java @@ -3807,7 +3807,7 @@ public boolean isViewReferenced() { MetaDataMutationResult result = connection.getQueryServices().createTable(tableMetaData, viewType == ViewType.MAPPED || allocateIndexId ? physicalNames.get(0).getBytes() : null, tableType, tableProps, familyPropList, splits, isNamespaceMapped, allocateIndexId, - UpgradeUtil.isNoUpgradeSet(connection.getClientInfo()), parent); + UpgradeUtil.isNoUpgradeSet(connection.getClientInfo()), parent, statement.ifNotExists()); MutationCode code = result.getMutationCode(); try { if (code != MutationCode.TABLE_NOT_FOUND) { diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AppendOnlySchemaIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AppendOnlySchemaIT.java index 61b615f0069..6cb467ad9f5 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AppendOnlySchemaIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AppendOnlySchemaIT.java @@ -117,7 +117,7 @@ private void testTableWithSameSchema(boolean notExists, boolean sameClient) thro // verify no create table rpcs verify(connectionQueryServices, never()).createTable(anyList(), any(byte[].class), any(PTableType.class), anyMap(), anyList(), any(byte[][].class), eq(false), eq(false), - eq(false), any(PTable.class)); + eq(false), any(PTable.class), eq(false)); reset(connectionQueryServices); // execute alter table ddl that adds the same column diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java index 42629cf4917..f01d37a118a 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java @@ -1771,7 +1771,7 @@ private int checkGuidePostWidth(String tableName) throws Exception { } @Test - public void testCreateTableDoesNotMutateExistingHBaseTableProperties() throws Exception { + public void testCreateTableWithIfNotExistsDoesNotMutateExistingHBaseTableProperties() throws Exception { String tableName = generateUniqueName(); Properties props = new Properties(); @@ -1805,26 +1805,47 @@ public void testCreateTableDoesNotMutateExistingHBaseTableProperties() throws Ex assertEquals("VERSIONS should not be modified by IF NOT EXISTS", 1, cfsAfter[0].getMaxVersions()); assertEquals("TTL should not be modified by IF NOT EXISTS", 86400, cfsAfter[0].getTimeToLive()); + } + + @Test + public void testCreateTableWithoutIfNotExistsShouldMutateExistingHBaseTableProperties() throws Exception { + String tableName = generateUniqueName(); + Properties props = new Properties(); + + // Step 1: Create table with VERSIONS=1 and TTL=86400 + String createDdl = "CREATE TABLE " + tableName + " (" + " PK VARCHAR NOT NULL PRIMARY KEY," + + " COL1 VARCHAR," + " COL2 INTEGER" + " ) VERSIONS=1, TTL=86400"; + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { + conn.createStatement().execute(createDdl); + } + + // Verify initial HBase table properties + Admin admin = driver.getConnectionQueryServices(getUrl(), props).getAdmin(); + TableDescriptor descBefore = admin.getDescriptor(TableName.valueOf(tableName)); + ColumnFamilyDescriptor[] cfsBefore = descBefore.getColumnFamilies(); + assertEquals(1, cfsBefore.length); + assertEquals(1, cfsBefore[0].getMaxVersions()); + assertEquals(86400, cfsBefore[0].getTimeToLive()); - // Step 4: Execute CREATE TABLE without IF NOT EXISTS with different properties + // Step 2: Execute CREATE TABLE without IF NOT EXISTS with different properties String createWithoutIfNotExistsDdl = - "CREATE TABLE " + tableName + " (" + " PK VARCHAR NOT NULL PRIMARY KEY," + " COL1 VARCHAR," - + " COL2 INTEGER" + " ) VERSIONS=6, TTL=120000"; + "CREATE TABLE " + tableName + " (" + " PK VARCHAR NOT NULL PRIMARY KEY," + " COL1 VARCHAR," + + " COL2 INTEGER" + " ) VERSIONS=6, TTL=120000"; try (Connection conn = DriverManager.getConnection(getUrl(), props)) { // Should fail as table exists already. TableAlreadyExistsException exception = assertThrows(TableAlreadyExistsException.class, - () -> conn.createStatement().execute(createWithoutIfNotExistsDdl)); - assertEquals("ERROR 1013 (42M04): Table already exists. tableName=N000063", - exception.getMessage()); + () -> conn.createStatement().execute(createWithoutIfNotExistsDdl)); + assertEquals("ERROR 1013 (42M04): Table already exists. tableName=" + tableName, + exception.getMessage()); } - // Step 5: Verify that HBase table properties are still NOT changed + // Step 3: Verify that HBase table properties are changed TableDescriptor descAfter2 = admin.getDescriptor(TableName.valueOf(tableName)); ColumnFamilyDescriptor[] cfsAfter2 = descAfter2.getColumnFamilies(); assertEquals(1, cfsAfter2.length); - assertEquals("VERSIONS should not be modified for existing table", 1, - cfsAfter2[0].getMaxVersions()); - assertEquals("TTL should not be modified for existing table", 86400, - cfsAfter2[0].getTimeToLive()); + assertEquals("VERSIONS should be modified without IF NOT EXISTS", 6, + cfsAfter2[0].getMaxVersions()); + assertEquals("TTL should be modified without IF NOT EXISTS", 120000, + cfsAfter2[0].getTimeToLive()); } }