rishi-rana commented on code in PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#discussion_r4117929238
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/ExtensionsTestBase.java:
##########
@@ -51,6 +51,7 @@ public static void startMetastoreAndSpark() {
.master("local[2]")
.config("spark.driver.host",
InetAddress.getLoopbackAddress().getHostAddress())
.config("spark.testing", "true")
+ .config("spark.sql.planChangeValidation", "true")
Review Comment:
I ran the full `spark-extensions` suite with
`spark.sql.planChangeValidation` enabled on each of the
other supported Spark versions:
| Spark | tests | failures | plan-validation failures |
|---|---|---|---|
| 4.0 | 2718 | 21 | 0 |
| 4.1 | 2893 | 0 | 0 |
| 4.2 | 2952 | 0 | 0 |
The 21 failures on 4.0 are all `TestRewriteTablePathProcedure` hitting a
local Hive metastore
(`TTransportException` / `MetaException`); that class passes when run on its
own. No version produced
a `PLAN_VALIDATION_FAILED_RULE_IN_BATCH`.
One thing worth knowing for the decision: `ExtensionsTestBase` sets
`spark.testing` as a Spark conf,
but Spark gates per-rule validation on `Utils.isTesting`, which reads the
`spark.testing` **system
property**. So these suites have never actually had validation on, which is
how the 3.5 issue here
went unnoticed.
So enabling it on 4.x is clean today and purely preventative — there is no
bug to fix there. Happy to
do it as a follow-up PR so this one stays on the 3.5 fix, or to fold it in
here if you'd rather have
it in one change.
--
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]
