Skip to content

[#12516] improvement(scripts): Add IF NOT EXISTS to CREATE INDEX in upgrade SQL scripts for idempotency - #12968

Open
LiJie20190102 wants to merge 1 commit into
apache:mainfrom
LiJie20190102:12516-idempotent-create-index
Open

[#12516] improvement(scripts): Add IF NOT EXISTS to CREATE INDEX in upgrade SQL scripts for idempotency#12968
LiJie20190102 wants to merge 1 commit into
apache:mainfrom
LiJie20190102:12516-idempotent-create-index

Conversation

@LiJie20190102

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

PostgreSQL: add IF NOT EXISTS to 5 CREATE INDEX statements in upgrade-1.1.0-to-1.2.0-postgresql.sql.

MySQL: use prepared statements with information_schema check to make 7 CREATE [UNIQUE] INDEX statements idempotent across 2 upgrade scripts (upgrade-1.2.0-to-1.3.0-mysql.sql and upgrade-1.3.0-to-2.0.0-mysql.sql), since MySQL 8.0 does not support CREATE INDEX IF NOT EXISTS.

Add testUpgradeSQLScriptIdempotency to TestSQLScripts to verify that re-running upgrade scripts does not fail on index-creation statements.

Why are the changes needed?

(Please clarify why the changes are needed. For instance,

  1. If you propose a new API, clarify the use case for a new API.
  2. If you fix a bug, describe the bug.)

Fix: #12516

Does this PR introduce any user-facing change?

(Please list the user-facing changes introduced by your change, including

  1. Change in user-facing APIs.
  2. Addition or removal of property keys.)

How was this patch tested?

Execute the method org.apache.gravitino.storage.TestSQLScripts#testUpgradeSQLScriptIdempotency

@yuqi1129

yuqi1129 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

@LiJie20190102
Please resolve the conflict.

…ade SQL scripts for idempotency

PostgreSQL: add IF NOT EXISTS to 5 CREATE INDEX statements in
upgrade-1.1.0-to-1.2.0-postgresql.sql.

MySQL: use prepared statements with information_schema check to make
7 CREATE [UNIQUE] INDEX statements idempotent across 2 upgrade scripts
(upgrade-1.2.0-to-1.3.0-mysql.sql and upgrade-1.3.0-to-2.0.0-mysql.sql),
since MySQL 8.0 does not support CREATE INDEX IF NOT EXISTS.

Add testUpgradeSQLScriptIdempotency to TestSQLScripts to verify that
re-running upgrade scripts does not fail on index-creation statements.
@LiJie20190102
LiJie20190102 force-pushed the 12516-idempotent-create-index branch from dd399c7 to bec300a Compare September 7, 2026 13:08
@LiJie20190102

Copy link
Copy Markdown
Contributor Author

@LiJie20190102 Please resolve the conflict.

Okay, it has been resolved

@yuqi1129

yuqi1129 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fix. I suggest keeping only the PostgreSQL changes in this PR and discussing the MySQL changes separately.

The MySQL checks can avoid creating the same index again, but other statements, such as ADD COLUMN and DROP INDEX, can still fail when we run the script again. So the extra SQL does not yet solve the upgrade retry problem. Could we first clarify which retry case we want to support?

For the test, please check that the indexes exist and have the expected columns and uniqueness. If we only want to test running the index statements twice, please make that scope clear. Ignoring errors from other DDL statements does not show that the whole upgrade script can be run again safely.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

Code Coverage Report

Overall Project 69.66% -0.31% 🟢
Files changed 44.03% 🔴

Module Coverage
aliyun 19.74% 🔴
api 51.57% 🟢
authorization-common 85.96% 🟢
authorization-ranger 4.38% 🔴
aws 53.54% 🟢
azure 32.1% 🔴
catalog-common 22.89% 🔴
catalog-fileset 82.17% 🟢
catalog-glue 69.8% 🟢
catalog-hive 82.96% 🟢
catalog-jdbc-common 45.09% 🟢
catalog-jdbc-doris 82.69% 🟢
catalog-jdbc-mysql 79.33% 🟢
catalog-jdbc-postgresql 83.83% 🟢
catalog-jdbc-starrocks 79.16% 🟢
catalog-kafka 76.99% 🟢
catalog-lakehouse-generic 60.88% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 85.9% 🟢
catalog-lakehouse-paimon 84.29% 🟢
catalog-model 77.99% 🟢
cli 44.51% 🟢
client-java 77.5% 🟢
common 56.89% 🟢
core 84.19% 🟢
filesystem-hadoop3 76.48% 🟢
flink 0.0% 🔴
flink-common 53.22% 🟢
flink-runtime 0.0% 🔴
gcp 32.2% 🔴
hadoop-auth 68.0% 🟢
hadoop-common 17.84% 🔴
hive-metastore-common 53.5% 🟢
iceberg-aliyun-bundle 0.0% 🔴
iceberg-common 66.58% 🟢
iceberg-rest-server 76.59% 🟢
idp-basic 87.1% 🟢
integration-test-common 0.0% 🔴
jobs 62.92% 🟢
lance-common 32.52% 🔴
lance-rest-server 68.12% 🟢
lineage 59.39% 🟢
optimizer 83.24% 🟢
optimizer-api 21.95% 🔴
server 90.11% 🟢
server-common 81.23% 🟢
spark 28.57% +28.57% 🔴
spark-common 51.8% -6.88% 🟢
tencent 81.78% 🟢
trino-connector 58.36% 🟢
Files
Module File Coverage
spark SparkTableChangeConverter34.java 87.5% 🟢
SparkHiveTypeConverter34.java 77.78% 🟢
SparkTypeConverter34.java 75.0% 🟢
GravitinoGlueCatalogSpark33.java 0.0% 🔴
GravitinoHiveCatalogSpark33.java 0.0% 🔴
GravitinoIcebergCatalogSpark33.java 0.0% 🔴
GravitinoJdbcCatalogSpark33.java 0.0% 🔴
GravitinoPostgreSqlCatalogSpark33.java 0.0% 🔴
GravitinoPaimonCatalogSpark33.java 0.0% 🔴
GravitinoGlueCatalogSpark34.java 0.0% 🔴
GravitinoHiveCatalogSpark34.java 0.0% 🔴
GravitinoIcebergCatalogSpark34.java 0.0% 🔴
GravitinoJdbcCatalogSpark34.java 0.0% 🔴
SparkJdbcTypeConverter34.java 0.0% 🔴
GravitinoPostgreSqlCatalogSpark34.java 0.0% 🔴
GravitinoPaimonCatalogSpark34.java 0.0% 🔴
GravitinoGlueCatalogSpark35.java 0.0% 🔴
GravitinoHiveCatalogSpark35.java 0.0% 🔴
GravitinoIcebergCatalogSpark35.java 0.0% 🔴
GravitinoJdbcCatalogSpark35.java 0.0% 🔴
GravitinoPaimonCatalogSpark35.java 0.0% 🔴
spark-common SparkTypeConverter.java 95.24% 🟢
AuthorizationTable.java 95.24% 🟢
SparkTableChangeConverter.java 92.19% 🟢
SparkPartitionUtils.java 88.89% 🟢
SparkJdbcTypeConverter.java 83.33% 🟢
PaimonPropertiesConverter.java 66.67% 🟢
GravitinoAuthorizationSparkSessionExtensions.java 52.94% 🔴
GravitinoDriverPlugin.java 41.77% 🔴
GravitinoIcebergCatalog.java 31.36% 🔴
BaseCatalog.java 21.17% 🔴
SparkHiveTypeConverter.java 14.29% 🔴
SparkHiveTable.java 0.0% 🔴
GravitinoPaimonCatalog.java 0.0% 🔴
PaimonPropertiesConstants.java 0.0% 🔴
SparkPaimonTable.java 0.0% 🔴
GravitinoSparkPlugin.java 0.0% 🔴
HiveGravitinoOperationOperator.java 0.0% 🔴
CatalogNameAdaptor.java 0.0% 🔴

@LiJie20190102

Copy link
Copy Markdown
Contributor Author

Thanks for the fix. I suggest keeping only the PostgreSQL changes in this PR and discussing the MySQL changes separately.

The MySQL checks can avoid creating the same index again, but other statements, such as ADD COLUMN and DROP INDEX, can still fail when we run the script again. So the extra SQL does not yet solve the upgrade retry problem. Could we first clarify which retry case we want to support?

For the test, please check that the indexes exist and have the expected columns and uniqueness. If we only want to test running the index statements twice, please make that scope clear. Ignoring errors from other DDL statements does not show that the whole upgrade script can be run again safely.

Thanks for the detailed review, @yuqi1129 . I've reverted the MySQL changes from this PR as you suggested.

Regarding the MySQL scope, the original issue #12516 only asks for CREATE INDEX idempotency — it does not cover ADD COLUMN, DROP INDEX, or RENAME INDEX. You're right that making only CREATE INDEX idempotent doesn't solve the full upgrade-retry problem for MySQL, since those other DDL statements would still fail on re-run.

I see two options:

Keep this PR PostgreSQL-only, and open a separate issue for full MySQL upgrade-script idempotency (covering ADD COLUMN, DROP INDEX, RENAME INDEX, etc.). This keeps the scope tight and matches the issue title.

Expand this PR to make all DDL in the MySQL upgrade scripts idempotent using stored procedures (AddColumnIfMissing, DropIndexIfExists, RenameIndexIfExists, CreateIndexIfNotExists). This would be a larger change and might be better as its own PR.

Which approach do you prefer? If you'd like a separate issue, I'm happy to file one.

@LiJie20190102 LiJie20190102 changed the title [#12516] improvement: Add IF NOT EXISTS to CREATE INDEX in upgrade SQ… [#12516] improvement(scripts): Add IF NOT EXISTS to CREATE INDEX in upgrade SQ… Sep 10, 2026
@LiJie20190102 LiJie20190102 changed the title [#12516] improvement(scripts): Add IF NOT EXISTS to CREATE INDEX in upgrade SQ… [#12516] improvement(scripts): Add IF NOT EXISTS to CREATE INDEX in upgrade SQL scripts for idempotency Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] Add IF NOT EXISTS to CREATE INDEX in upgrade SQL scripts for idempotency

2 participants