github-actions[bot] commented on code in PR #63521:
URL: https://github.com/apache/doris/pull/63521#discussion_r3465302434
##########
regression-test/suites/external_table_p0/paimon/test_paimon_gcs.groovy:
##########
@@ -55,12 +69,7 @@ suite("test_paimon_gcs", "p0,external") {
sql """drop catalog if exists ${catalog_name}_with_region"""
sql """
CREATE CATALOG ${catalog_name}_with_region PROPERTIES (
- 'type' = 'paimon',
- 'warehouse' = '${gcs_warehouse}',
- '${propertyPrefix}.endpoint' = '${gcs_endpoint}',
Review Comment:
The new `withRegion` variant is not actually configuring a region for the
GCS catalog. Because the catalog also sets `fs.gcs.support=true`, storage
detection selects `GCSProperties`, but that class keeps `region` hard-coded to
`us-east1` and does not bind `gs.region`, `s3.region`, or `GCSRegion` through a
`@ConnectorProperty`. The Paimon catalog option path only forwards `paimon.*`
keys, so this property is dropped there too. As a result changing `GCSRegion`
in the external pipeline does not change the FE/BE Hadoop config, and this half
of the regression is effectively the same as the no-region case. Please either
wire the region aliases into the GCS storage properties path, or remove this
region variant so the test does not claim coverage it does not provide.
##########
regression-test/suites/external_table_p0/paimon/test_paimon_gcs.groovy:
##########
@@ -26,15 +26,29 @@ suite("test_paimon_gcs", "p0,external") {
String table_name = "hive_test_table"
for (String propertyPrefix : ["gs", "s3"]) {
+ def catalogProperties = { boolean withRegion ->
+ def properties = [
+ "'type' = 'paimon'",
+ "'warehouse' = '${gcs_warehouse}'",
+ "'fs.gcs.support' = 'true'",
+ ]
+ def optionalProperties = [
+ [gcs_endpoint, "'${propertyPrefix}.endpoint' =
'${gcs_endpoint}'"],
+ [gcs_ak, "'${propertyPrefix}.access_key' =
'${gcs_ak}'"],
+ [gcs_sk, "'${propertyPrefix}.secret_key' =
'${gcs_sk}'"],
+ ]
+ if (withRegion) {
+ optionalProperties.add([gcs_region,
"'${propertyPrefix}.region' = '${gcs_region}'"])
+ }
+ properties.addAll(optionalProperties.findAll { it[0] != null
&& !it[0].trim().isEmpty() }
+ .collect { it[1] })
+ properties.join(",\n")
+ }
Review Comment:
Re-enabling this suite as the P0 GCS regression still never exercises the
`gs://` path. Both `gs` and `s3` prefix iterations reuse this `s3://...`
warehouse, so the test only proves that the S3A-compatible URI can reach the
GCS endpoint. Doris has distinct GCS scheme handling (`SchemaTypeMapper` maps
`gs` to GCS, and `GCSProperties` installs
`fs.gs.impl`/`fs.gs.impl.disable.cache`), and the existing HMS Paimon GCS test
uses `gs://...` for its GCS warehouse. Please make at least one catalog here
use `gs://selectdb-qa-datalake-test/paimon_warehouse` so this newly enabled P0
case actually guards the GCS URI scheme as well as the S3-compatible alias.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
