LuciferYang commented on code in PR #12525:
URL: https://github.com/apache/gluten/pull/12525#discussion_r3587883549
##########
docs/velox-backend-limitations.md:
##########
@@ -6,12 +6,12 @@ nav_order: 5
This document describes the limitations of velox backend by listing some known
cases where exception will be thrown, gluten behaves incompatibly with spark,
or certain plan's execution
must fall back to vanilla spark, etc.
-### Override of Spark classes (For Spark3.2 and Spark3.3)
+### Override of Spark classes (For Spark3.3)
Review Comment:
Reworded the header to be generic ('Override of Spark classes') since the
mechanism applies across all supported Spark versions. Applied in the latest
force-push.
##########
docs/velox-backend-limitations.md:
##########
@@ -6,12 +6,12 @@ nav_order: 5
This document describes the limitations of velox backend by listing some known
cases where exception will be thrown, gluten behaves incompatibly with spark,
or certain plan's execution
must fall back to vanilla spark, etc.
-### Override of Spark classes (For Spark3.2 and Spark3.3)
+### Override of Spark classes (For Spark3.3)
Gluten avoids to modify Spark's existing code and use Spark APIs if possible.
However, some APIs aren't exposed in Vanilla spark and we have to copy the
Spark file and do the hardcode changes. The list of override classes can be
found as ignoreClasses in package/pom.xml . If you use customized Spark, you
may check if the files are modified in your spark, otherwise your changes will
be overrided.
So you need to ensure preferentially load the Gluten jar to overwrite the jar
of vanilla spark. Refer to [How to prioritize loading Gluten jars in
Spark](https://github.com/apache/gluten/blob/main/docs/velox-backend-troubleshooting.md#incompatible-class-error-when-using-native-writer).
-If not officially supported spark3.2/3.3 version is used, NoSuchMethodError
can be thrown at runtime. More details see
[issue-4514](https://github.com/apache/gluten/issues/4514).
+If not officially supported spark3.3 version is used, NoSuchMethodError can be
thrown at runtime. More details see
[issue-4514](https://github.com/apache/gluten/issues/4514).
Review Comment:
Reworded to 'If an unofficially supported Spark version is used' to avoid
tying the warning to a specific Spark minor. Applied in the latest force-push.
##########
docs/velox-backend-limitations.md:
##########
@@ -6,12 +6,12 @@ nav_order: 5
This document describes the limitations of velox backend by listing some known
cases where exception will be thrown, gluten behaves incompatibly with spark,
or certain plan's execution
must fall back to vanilla spark, etc.
-### Override of Spark classes (For Spark3.2 and Spark3.3)
+### Override of Spark classes (For Spark3.3)
Gluten avoids to modify Spark's existing code and use Spark APIs if possible.
However, some APIs aren't exposed in Vanilla spark and we have to copy the
Spark file and do the hardcode changes. The list of override classes can be
found as ignoreClasses in package/pom.xml . If you use customized Spark, you
may check if the files are modified in your spark, otherwise your changes will
be overrided.
Review Comment:
Fixed the typo 'overrided' -> 'overridden' since the line was already
touched by this cleanup. Applied in the latest force-push.
##########
shims/spark33/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcFileFormat.scala:
##########
@@ -44,11 +44,10 @@ import java.io._
import java.net.URI
/* -
- * This class is copied from Spark 3.2 and modified for Gluten. \n
+ * This class is copied from Spark and modified for Gluten. \n
* Gluten should make sure this class is loaded before the original class.
* If new Spark releases accepts changes and make this class incompatible,
- * we can move this class to shims-spark32,
- * shims-spark33, etc.
+ * we can move this class to a specific shim (shims-spark33, etc.).
Review Comment:
Rewrote the ScalaDoc to remove the literal '\\n' and clean up the grammar
('If a new Spark release accepts changes that make this class incompatible
...'). Applied in the latest force-push.
##########
tools/gluten-it/common/src/main/java/org/apache/gluten/integration/SparkJvmOptions.java:
##########
@@ -28,10 +28,10 @@ public static String read() {
final Class<?> clazz =
Class.forName("org.apache.spark.launcher.JavaModuleOptions");
final Method method = clazz.getMethod("defaultModuleOptions");
return (String) method.invoke(null);
- } catch (ClassNotFoundException e) {
- // Could happen in Spark 3.2 which doesn't have this class yet.
- return "";
- } catch (NoSuchMethodException | InvocationTargetException |
IllegalAccessException e) {
+ } catch (ClassNotFoundException
Review Comment:
Now Class.forName uses the MODULE_OPTIONS_CLASS_NAME constant instead of
duplicating the string literal. Applied in the latest force-push.
--
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]
