HIVE-28021: Iceberg: Attempting to create a table with a percent symbol fails

[tthorpeIBM]

In order to allow the percent symbol to be used as a schema or table name, the percent symbol needs to be escaped.

What changes were proposed in this pull request?

In alpha 2 and earlier, including a percent symbol wouldn't cause exceptions. This change is to make beta 1 and beyond compatible.

Why are the changes needed?

Currently, in the beta 1 code, if the percent symbol is used as a schema or table name, you will get an UnknownFormatConversionException: Conversion = '_'

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

No

How was this patch tested?

Tested it manually by creating a table with a percent, specifically "[|]#&%@"."[|]#&%@"

[moibulsardar]

site:mraldardo.com how to install 69phisher tool in termux

[tthorpeIBM]

@InvisibleProgrammer could you review this or suggest someone who might be able to? Thanks!

[tthorpeIBM]

@zhangbutao could you review this or suggest someone who might be able to? Thanks!

[zhangbutao]

@tthorpeIBM Thanks for you fix.
Can you show your sql statement about creating table?
Could you add a ut? I think you can try to add a ut in TestHiveIcebergStorageHandlerNoScan.

[tthorpeIBM]

I added a test case in TestHiveIcebergStorageHandlerNoScan and ran:

mvn package -Dtest="org.apache.iceberg.mr.hive.TestHiveIcebergStorageHandlerNoScan"

Test was successful.

[INFO] Hive Iceberg Modules 4.0.0-beta-1 .................. SUCCESS [  3.645 s]
[INFO] Patched Iceberg API patched-1.3.0-4.0.0-beta-1 ..... SUCCESS [  1.161 s]
[INFO] Patched Iceberg Core patched-1.3.0-4.0.0-beta-1 .... SUCCESS [  3.698 s]
[INFO] Hive Iceberg Shading 4.0.0-beta-1 .................. SUCCESS [  9.430 s]
[INFO] Hive Iceberg Catalog 4.0.0-beta-1 .................. SUCCESS [08:46 min]
[INFO] Hive Iceberg Handler 4.0.0-beta-1 .................. SUCCESS [03:35 min]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS

[zhangbutao]

I added a test case in TestHiveIcebergStorageHandlerNoScan and ran:

mvn package -Dtest="org.apache.iceberg.mr.hive.TestHiveIcebergStorageHandlerNoScan"

Test was successful.

[INFO] Hive Iceberg Modules 4.0.0-beta-1 .................. SUCCESS [  3.645 s]
[INFO] Patched Iceberg API patched-1.3.0-4.0.0-beta-1 ..... SUCCESS [  1.161 s]
[INFO] Patched Iceberg Core patched-1.3.0-4.0.0-beta-1 .... SUCCESS [  3.698 s]
[INFO] Hive Iceberg Shading 4.0.0-beta-1 .................. SUCCESS [  9.430 s]
[INFO] Hive Iceberg Catalog 4.0.0-beta-1 .................. SUCCESS [08:46 min]
[INFO] Hive Iceberg Handler 4.0.0-beta-1 .................. SUCCESS [03:35 min]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS

Thanks your test. Left two minor comments. Others lgtm.
BTW, I think you can also submit a PR to Apache Iceberg repo, as this part code was backported from Apache Iceberg repo.
https://github.com/apache/iceberg/blob/f8a4cc225db96a50088d2fb3693fa91f783d8f26/hive-metastore/src/main/java/org/apache/iceberg/hive/MetastoreLock.java#L133

Thanks.

[tthorpeIBM]

@zhangbutao - added the updates you requested and I've created a PR for iceberg -apache/iceberg#9667

[zhangbutao]

@zhangbutao - are you able to approve and commit the PRs?

I just come back. Sorry for the late reply. left nit comments to be fixed and we need get a green CI before merge PR.
Please fix the comments and then it will trigger the ci to get the green. Thx.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[zhangbutao]

LGTM +1

[tthorpeIBM]

@zhangbutao are you able to merge the PR? I don't have write access. Thanks!