You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2024/05/10 06:23:35 UTC

[PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

pan3793 opened a new pull request, #46521:
URL: https://github.com/apache/spark/pull/46521

   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
     8. If you want to add or modify an error type or message, please read the guideline first in
        'common/utils/src/main/resources/error/README.md'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   CodeHaus Jackson dependencies were pulled from Hive, while in https://github.com/apache/hive/pull/4564 (Hive 2.3.10), it migrated to Jackson 2.x, so we can remove them from Spark now.
   
   ### 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, you can clarify why it is a bug.
   -->
   Remove unused and vulnerable dependencies.
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   No
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
   -->
   Pass GA.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   <!--
   If generative AI tooling has been used in the process of authoring this patch, please include the
   phrase: 'Generated-by: ' followed by the name of the tool and its version.
   If no, write 'No'.
   Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
   -->
   No.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2706965438

   > For this one PR, I believe we need a verification for different HMS versions to make it sure.
   
   @dongjoon-hyun I managed to set up an env to test the IsolatedClassLoader, it works as expected.
   
   The basic test steps:
   - Hadoop Cluster with HDFS and YARN (v3.3.6)
   - HMS 3.1.2
   - Spark 4.0.0 RC2 and delete `hive-jacksons` folder
   
   
   Verify built-in Hive 2.3.10 works well without CodeHaus Jackson jars
   
   ```
   $ bin/spark-shell
   scala> spark.sql("show databases").show()
   +---------+
   |namespace|
   +---------+
   |  default|
   +---------+
   
   scala> spark.sql("create temporary function hive_uuid as 'org.apache.hadoop.hive.ql.udf.UDFUUID'")
   val res1: org.apache.spark.sql.DataFrame = []
   
   scala> spark.sql("select hive_uuid()").show
   +--------------------+
   |         hive_uuid()|
   +--------------------+
   |3ad7e110-2ad9-4f0...|
   +--------------------+
   ```
   
   Verify Hive 3.1.3 metastore jars also works well without CodeHaus Jackson jars
   
   ```
   $ bin/spark-shell --conf spark.sql.hive.metastore.version=3.1.3 --conf spark.sql.hive.metastore.jars=maven
   scala> spark.sql("show databases").show()
   ... (triggers jar downloading include CodeHaus Jackson jars)
   downloading https://repo1.maven.org/maven2/org/codehaus/jackson/jackson-core-asl/1.9.13/jackson-core-asl-1.9.13.jar ...
   	[SUCCESSFUL ] org.codehaus.jackson#jackson-core-asl;1.9.13!jackson-core-asl.jar (1178ms)
   downloading https://repo1.maven.org/maven2/org/codehaus/jackson/jackson-mapper-asl/1.9.13/jackson-mapper-asl-1.9.13.jar ...
   	[SUCCESSFUL ] org.codehaus.jackson#jackson-mapper-asl;1.9.13!jackson-mapper-asl.jar (3245ms)
   downloading https://repo1.maven.org/maven2/org/codehaus/jackson/jackson-jaxrs/1.9.13/jackson-jaxrs-1.9.13.jar ...
   	[SUCCESSFUL ] org.codehaus.jackson#jackson-jaxrs;1.9.13!jackson-jaxrs.jar (429ms)
   downloading https://repo1.maven.org/maven2/org/codehaus/jackson/jackson-xc/1.9.13/jackson-xc-1.9.13.jar ...
   	[SUCCESSFUL ] org.codehaus.jackson#jackson-xc;1.9.13!jackson-xc.jar (454ms)
   ...
   downloading https://repo1.maven.org/maven2/org/apache/hive/hive-llap-client/3.1.3/hive-llap-client-3.1.3.jar ...
   	[SUCCESSFUL ] org.apache.hive#hive-llap-client;3.1.3!hive-llap-client.jar (484ms)
   downloading https://repo1.maven.org/maven2/org/apache/hive/hive-llap-common/3.1.3/hive-llap-common-3.1.3.jar ...
   	[SUCCESSFUL ] org.apache.hive#hive-llap-common;3.1.3!hive-llap-common.jar (851ms)
   ...
   :: retrieving :: org.apache.spark#spark-submit-parent-a3e58de0-7045-4d08-ba86-3d7b1fc03a46
   	confs: [default]
   	209 artifacts copied, 0 already retrieved (236164kB/372ms)
   2025-03-07T16:51:48.371441489Z main INFO Starting configuration XmlConfiguration[location=/etc/spark/conf/log4j2.xml, lastModified=2025-03-07T15:59:46.435Z]...
   2025-03-07T16:51:48.371577157Z main INFO Start watching for changes to /etc/spark/conf/log4j2.xml every 0 seconds
   2025-03-07T16:51:48.371673867Z main INFO Configuration XmlConfiguration[location=/etc/spark/conf/log4j2.xml, lastModified=2025-03-07T15:59:46.435Z] started.
   2025-03-07T16:51:48.371843828Z main INFO Stopping configuration org.apache.logging.log4j.core.config.DefaultConfiguration@4926e32c...
   2025-03-07T16:51:48.371982996Z main INFO Configuration org.apache.logging.log4j.core.config.DefaultConfiguration@4926e32c stopped.
   2025-03-07 16:51:48 INFO HiveConf: Found configuration file file:/etc/hive/conf/hive-site.xml
   Hive Session ID = 1bd666c3-8064-43e5-9b8d-5b680ccc5e6a
   2025-03-07 16:51:48 INFO SessionState: Hive Session ID = 1bd666c3-8064-43e5-9b8d-5b680ccc5e6a
   2025-03-07 16:51:48 INFO HiveMetaStoreClient: Trying to connect to metastore with URI thrift://hadoop-master1.orb.local:9083
   2025-03-07 16:51:48 INFO HiveMetaStoreClient: Opened a connection to metastore, current connections: 1
   2025-03-07 16:51:48 INFO HiveMetaStoreClient: Connected to metastore.
   2025-03-07 16:51:48 INFO RetryingMetaStoreClient: RetryingMetaStoreClient proxy=class org.apache.hadoop.hive.ql.metadata.SessionHiveMetaStoreClient ugi=root (auth:SIMPLE) retries=1 delay=1 lifetime=0
   +---------+
   |namespace|
   +---------+
   +---------+
   
   scala> spark.sql("create temporary function hive_uuid as 'org.apache.hadoop.hive.ql.udf.UDFUUID'")
   val res0: org.apache.spark.sql.DataFrame = []
   
   scala> spark.sql("select hive_uuid()").show
   +--------------------+
   |         hive_uuid()|
   +--------------------+
   |8ec74979-7fba-47e...|
   +--------------------+
   ```


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed pull request #46521: [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies
URL: https://github.com/apache/spark/pull/46521


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2299980914

   We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
   If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104034238

   I think the case is already covered by CI.
   
   When IsolatedClassLoader is enabled, the `Hive.get` should pull and load CodeHaus Jackson classes from IsolatedClassLoader. 
   
   
   ```
     /**
      * Initialize Hive through Configuration.
      * First try to use getWithoutRegisterFns to initialize to avoid loading all functions,
      * if there is no such method, fallback to Hive.get.
      */
     def getHive(conf: Configuration): Hive = {
       val hiveConf = conf match {
         case hiveConf: HiveConf =>
           hiveConf
         case _ =>
           new HiveConf(conf, classOf[HiveConf])
       }
       try {
         Hive.getWithoutRegisterFns(hiveConf)
       } catch {
         // SPARK-37069: not all Hive versions have the above method (e.g., Hive 2.3.9 has it but
         // 2.3.8 don't), therefore here we fallback when encountering the exception.
         case _: NoSuchMethodError =>
           Hive.get(hiveConf)
       }
   ```


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2707075252

   No, what I meant here was that your concern is legitimate. So, you can raise your concerns to the broader audience, @pan3793 . For example, dev@spark instead of this PR which is completely opposite to your intention.
   
   If you want, you can block Apache Spark 4.0.0 RCs by vetoing and initiating a discussion thread to add them all back.
   > In short, my conclusion is, we should and must keep all jars required by Hive built-in UDF 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2707065925

   > > For this one PR, I believe we need a verification for different HMS versions to make it sure.
   > 
   > that's a valid concern, since Spark CI only covers embedded HMS client case, let me test it with the real setup.
   
   I just found I forgot this stuff, and I think this PR could be reopened, so the comments should be visible to future reviewers.
   
   
   > I hope we can revisit this with you for Apache Spark 4.1.0 timeframe
   
   But given this should be done in 4.1, so let's focus on SPARK-51029 and move discussion to https://github.com/apache/spark/pull/49725 for now?
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2103948172

   cc @dongjoon-hyun and @wangyum 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104179423

   also cc @wangyum @yaooqinn @AngersZhuuuu @cloud-fan


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2706974909

   Thank you for checking, @pan3793 .
   
   Are you assuming to rebuild all Hive UDF jars here? I'm wondering if you are presenting the test result with old Hive built-UDF jars here.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2707031841

   In short, my conclusion is, we should and must keep all jars required by Hive built-in UDF to allow `o.a.h.hive.ql.exec.FunctionRegistry` initialization, for other jars like commons-lang, codehaus jackson, jodd, just remove them and let user add explicitly if required. (in my company's production cases, it's rare.)


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #46521:
URL: https://github.com/apache/spark/pull/46521#discussion_r1596337751


##########
pom.xml:
##########
@@ -1938,19 +1937,6 @@
           </exclusion>
         </exclusions>
       </dependency>
-      <!-- Hive 2.3 need this to init Hive's FunctionRegistry -->
-      <dependency>
-        <groupId>org.codehaus.jackson</groupId>
-        <artifactId>jackson-core-asl</artifactId>
-        <version>${codehaus.jackson.version}</version>
-        <scope>${hive.jackson.scope}</scope>

Review Comment:
   if we identified some issues on hive 2.3.10 before 4.0.0 release, we may need to revert this patch and fallback to SPARK-47119 approach to mitigate CodeHaus Jackson dependencies vulnerabilities, see comemnts at
   https://github.com/apache/spark/pull/45201#issuecomment-2103839853



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104071843

   @dongjoon-hyun AFAIK, the "Hive UDF execution" always uses built-in Hive jars without IsolatedClassLoader. While "Hive UDF registration" will happen during `Hive.get(hiveConf)` with IsolatedClassLoader on constructing HMS client.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104151119

   > @dongjoon-hyun AFAIK, the "Hive UDF execution" always uses built-in Hive jars without IsolatedClassLoader. While "Hive UDF registration" will happen during `Hive.get(hiveConf)` with IsolatedClassLoader on constructing HMS client.
   
   It sounds like that we could have a corner case. That's the reason why we need an actual test case to cover it, isn't it?


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2707102450

   Thank you, @pan3793 ! And, sorry for your inconvenience.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #46521:
URL: https://github.com/apache/spark/pull/46521#discussion_r1596329108


##########
pom.xml:
##########
@@ -1938,19 +1937,6 @@
           </exclusion>
         </exclusions>
       </dependency>
-      <!-- Hive 2.3 need this to init Hive's FunctionRegistry -->
-      <dependency>
-        <groupId>org.codehaus.jackson</groupId>
-        <artifactId>jackson-core-asl</artifactId>
-        <version>${codehaus.jackson.version}</version>
-        <scope>${hive.jackson.scope}</scope>

Review Comment:
   Can we also remove `<hive.jackson.scope>compile</hive.jackson.scope>`?
   https://github.com/apache/spark/blob/44f00ccebfd2b55862212dc2ac6c0c5b03c86227/pom.xml#L270
   https://github.com/apache/spark/blob/2df494fd4e4e64b9357307fb0c5e8fc1b7491ac3/assembly/pom.xml#L269
   https://github.com/apache/spark/blob/master/assembly/pom.xml#L272-L277



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104188970

   > For this one PR, I believe we need a verification for different HMS versions to make it sure.
   
   that's a valid concern, since Spark CI only covers embedded HMS client case, let me test it with the real setup.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #46521:
URL: https://github.com/apache/spark/pull/46521#discussion_r1596337751


##########
pom.xml:
##########
@@ -1938,19 +1937,6 @@
           </exclusion>
         </exclusions>
       </dependency>
-      <!-- Hive 2.3 need this to init Hive's FunctionRegistry -->
-      <dependency>
-        <groupId>org.codehaus.jackson</groupId>
-        <artifactId>jackson-core-asl</artifactId>
-        <version>${codehaus.jackson.version}</version>
-        <scope>${hive.jackson.scope}</scope>

Review Comment:
   if we identify some issues on hive 2.3.10 before 4.0.0 release, we may need to revert this patch and fallback to SPARK-47119 approach to mitigate CodeHaus Jackson dependencies vulnerabilities, see comemnts at
   https://github.com/apache/spark/pull/45201#issuecomment-2103839853



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104025408

   I have one separate question (or request), @wangyum and @pan3793 .
   
   When we use `Isolated Class Loader` to use old Hive client and old version HMS server and Hive UDF (requires Jackson).
   
   Will everything will work without any issues? Do you think we can have a test coverage for that?


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #46521:
URL: https://github.com/apache/spark/pull/46521#discussion_r2161995816


##########
pom.xml:
##########
@@ -1938,19 +1937,6 @@
           </exclusion>
         </exclusions>
       </dependency>
-      <!-- Hive 2.3 need this to init Hive's FunctionRegistry -->
-      <dependency>
-        <groupId>org.codehaus.jackson</groupId>
-        <artifactId>jackson-core-asl</artifactId>
-        <version>${codehaus.jackson.version}</version>
-        <scope>${hive.jackson.scope}</scope>

Review Comment:
   removed



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2996416737

   Now that Spark 4.0.0 has been successfully released with Hive 2.3.10, I think we can continue the process of removing CodeHaus Jackson dependencies.
   @wangyum @dongjoon-hyun can you please help re-open this PR?


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104039941

   Ya, I know that part, but do we have an end-to-end Hive UDF registration and invocation test case?


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #46521:
URL: https://github.com/apache/spark/pull/46521#discussion_r1596339218


##########
pom.xml:
##########
@@ -1938,19 +1937,6 @@
           </exclusion>
         </exclusions>
       </dependency>
-      <!-- Hive 2.3 need this to init Hive's FunctionRegistry -->
-      <dependency>
-        <groupId>org.codehaus.jackson</groupId>
-        <artifactId>jackson-core-asl</artifactId>
-        <version>${codehaus.jackson.version}</version>
-        <scope>${hive.jackson.scope}</scope>

Review Comment:
   ok



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2706983644

   I added this to a subtask of SPARK-48231 . 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2707044958

   > In short, my conclusion is, we should and must keep all jars required by Hive built-in UDF to allow `o.a.h.hive.ql.exec.FunctionRegistry` initialization, for other jars like commons-lang, codehaus jackson, jodd, just remove them and let user add explicitly if required. (in my company's production cases, it's rare.)
   
   I fully understand your backgrounds, reasoning, and this conclusion. May I ask why you initiate that discussion on this PR, `[SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies`, @pan3793 ?


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2707077185

   The RC is supposed to gather those kind of feedbacks and difficulties. There is no Apache Spark 4.0.0 until we have a community-blessed one.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #46521:
URL: https://github.com/apache/spark/pull/46521#discussion_r1596329108


##########
pom.xml:
##########
@@ -1938,19 +1937,6 @@
           </exclusion>
         </exclusions>
       </dependency>
-      <!-- Hive 2.3 need this to init Hive's FunctionRegistry -->
-      <dependency>
-        <groupId>org.codehaus.jackson</groupId>
-        <artifactId>jackson-core-asl</artifactId>
-        <version>${codehaus.jackson.version}</version>
-        <scope>${hive.jackson.scope}</scope>

Review Comment:
   Can we also remove `<hive.jackson.scope>compile</hive.jackson.scope>`?
   https://github.com/apache/spark/blob/44f00ccebfd2b55862212dc2ac6c0c5b03c86227/pom.xml#L270
   https://github.com/apache/spark/blob/2df494fd4e4e64b9357307fb0c5e8fc1b7491ac3/assembly/pom.xml#L269



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #46521:
URL: https://github.com/apache/spark/pull/46521#discussion_r1596329108


##########
pom.xml:
##########
@@ -1938,19 +1937,6 @@
           </exclusion>
         </exclusions>
       </dependency>
-      <!-- Hive 2.3 need this to init Hive's FunctionRegistry -->
-      <dependency>
-        <groupId>org.codehaus.jackson</groupId>
-        <artifactId>jackson-core-asl</artifactId>
-        <version>${codehaus.jackson.version}</version>
-        <scope>${hive.jackson.scope}</scope>

Review Comment:
   Can we also remove `<hive.jackson.scope>compile</hive.jackson.scope>`?
   https://github.com/apache/spark/blob/44f00ccebfd2b55862212dc2ac6c0c5b03c86227/pom.xml#L270



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #46521:
URL: https://github.com/apache/spark/pull/46521#discussion_r1596346293


##########
pom.xml:
##########
@@ -1938,19 +1937,6 @@
           </exclusion>
         </exclusions>
       </dependency>
-      <!-- Hive 2.3 need this to init Hive's FunctionRegistry -->
-      <dependency>
-        <groupId>org.codehaus.jackson</groupId>
-        <artifactId>jackson-core-asl</artifactId>
-        <version>${codehaus.jackson.version}</version>
-        <scope>${hive.jackson.scope}</scope>

Review Comment:
   Ya, sorry for making things difficult, @pan3793 and @wangyum .
   
   If we are sure, we can clean up more easily definitely.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104725585

   Thank you. Please attach the test results to the PR description.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2706978635

   BTW, thank you for taking a look at removing this.
   I support your direction and I hope we can revisit this with you for Apache Spark 4.1.0 timeframe, @pan3793 .


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2707020792

   > Are you assuming to rebuild all Hive UDF jars here?
   
   @dongjoon-hyun I never made such an assumption, most of the existing UDFs should work without any change, except to: the UDFs explicitly imports and uses the classes we removed from Spark new releases, it is not limited to CodeHaus Jackson, the risk happens each time when we update `dev/deps/spark-deps-hadoop-3-hive-2.3`. 
   
   let's say if the CustomUDF built with Hive 2.3.9 uses OkHTTP classes, it works well in Spark 3.5 because it ships OkHTTP jar by K8s client 6, but Spark 4.0 removes OkHTTP jars during K8s client 7 upgrading, then the CustomUDF should fail with OkHTTP class not found, by fixing it, the user can either shading the deps or add them by `--pacakges`(in this case, no rebuilt required because the Hive UDF interface is binary compatible), so it's user's responsibility to handle the UDF's transitive deps.
   
   What matters is that we must NOT break the Hive built-in UDF deps, otherwise, it blocks `o.a.h.hive.ql.exec.FunctionRegistry` initialization, and breaks the whole Hive UDF feature, that's why I argue that SPARK-51029 should be reverted.
   
   SPARK-51029 (GitHub PR [1]) removes `hive-llap-common` from the Spark binary distributions, which technically
   breaks the feature "Spark SQL supports integration of Hive UDFs, UDAFs and UDTFs"[2], more precisely, it change
   Hive UDF support from batteries included to not.
   
   In details, when user runs a query like `CREATE TEMPORARY FUNCTION hello AS 'my.HelloUDF'`, it triggers
   `o.a.h.hive.ql.exec.FunctionRegistry` initialization, which also initializes the Hive built-in UDFs, UDAFs and
   UDTFs[3], then NoClassDefFoundError ocuurs due to some built-in UDTFs depend on class in `hive-llap-common`. 
   
   ```
   org.apache.spark.sql.execution.QueryExecutionException: java.lang.NoClassDefFoundError: org/apache/hadoop/hive/llap/security/LlapSigner$Signable
   	at java.base/java.lang.Class.getDeclaredConstructors0(Native Method)
   	at java.base/java.lang.Class.privateGetDeclaredConstructors(Class.java:3373)
   	at java.base/java.lang.Class.getConstructor0(Class.java:3578)
   	at java.base/java.lang.Class.getDeclaredConstructor(Class.java:2754)
   	at org.apache.hive.common.util.ReflectionUtil.newInstance(ReflectionUtil.java:79)
   	at org.apache.hadoop.hive.ql.exec.Registry.registerGenericUDTF(Registry.java:208)
   	at org.apache.hadoop.hive.ql.exec.Registry.registerGenericUDTF(Registry.java:201)
   	at org.apache.hadoop.hive.ql.exec.FunctionRegistry.<clinit>(FunctionRegistry.java:500)
   	at org.apache.hadoop.hive.ql.udf.generic.GenericUDF.initializeAndFoldConstants(GenericUDF.java:160)
   	at org.apache.spark.sql.hive.HiveGenericUDFEvaluator.returnInspector$lzycompute(hiveUDFEvaluators.scala:118)
   	at org.apache.spark.sql.hive.HiveGenericUDFEvaluator.returnInspector(hiveUDFEvaluators.scala:117)
   	at org.apache.spark.sql.hive.HiveGenericUDF.dataType$lzycompute(hiveUDFs.scala:132)
   	at org.apache.spark.sql.hive.HiveGenericUDF.dataType(hiveUDFs.scala:132)
   	at org.apache.spark.sql.hive.HiveUDFExpressionBuilder$.makeHiveFunctionExpression(HiveSessionStateBuilder.scala:197)
   	at org.apache.spark.sql.hive.HiveUDFExpressionBuilder$.$anonfun$makeExpression$1(HiveSessionStateBuilder.scala:177)
   	at org.apache.spark.util.Utils$.withContextClassLoader(Utils.scala:187)
   	at org.apache.spark.sql.hive.HiveUDFExpressionBuilder$.makeExpression(HiveSessionStateBuilder.scala:171)
   	at org.apache.spark.sql.catalyst.catalog.SessionCatalog.$anonfun$makeFunctionBuilder$1(SessionCatalog.scala:1689)
       ...
   ```
   
   Currently (v4.0.0-rc2), user must add the `hive-llap-common` jar explicitly, e.g. by using
   `--packages org.apache.hive:hive-llap-common:2.3.10`, to fix the NoClassDefFoundError issue, even the `my.HelloUDF`
   does not depend on any class in `hive-llap-common`, this is quite confusing.
   
   [1] https://github.com/apache/spark/pull/49725
   [2] https://spark.apache.org/docs/3.5.5/sql-ref-functions-udf-hive.html
   [3] https://github.com/apache/hive/blob/rel/release-2.3.10/ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java#L208
   
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2998656550

   Please give me two hours to carry out some verification work


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104154946

   For this one PR, I believe we need a verification for different HMS versions to make it sure.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #46521:
URL: https://github.com/apache/spark/pull/46521#discussion_r1596346293


##########
pom.xml:
##########
@@ -1938,19 +1937,6 @@
           </exclusion>
         </exclusions>
       </dependency>
-      <!-- Hive 2.3 need this to init Hive's FunctionRegistry -->
-      <dependency>
-        <groupId>org.codehaus.jackson</groupId>
-        <artifactId>jackson-core-asl</artifactId>
-        <version>${codehaus.jackson.version}</version>
-        <scope>${hive.jackson.scope}</scope>

Review Comment:
   Ya, sorry for making things difficult, @pan3793 and @wangyum .
   
   If we are sure, we can clean up later more easily definitely.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2104175525

   Hmm, let me clear my view.
   
   In short, I think the current CI is sufficient.
   
   Spark uses Hive in two cases:
   
   1. As an HMS client. To support different HMS versions, it allows to use IsolatedClassLoder to load a different Hive class. It calls `Hive.getWithoutRegisterFns(hiveConf)` or `Hive.get(hiveConf)` to create the Hive instance, and there is a chance to trigger the Hive built-in UDF registration, for older Hive, e.g. 2.1.1, some built-in Hive UDF may trigger CodeHaus Jackson classes loading.
   
   2. As an execution library. Spark always used the built-in Hive jars to read/write Hive tables, execute Hive UDFs.
   
   For case 1, the CI already covers that(any older HMS client initialization triggers built-in UDF registration). For case 2, there is no chance to invoke CodeHaus Jackson classes since Hive 2.3.10 totally removed it in the codebase.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #46521: [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies
URL: https://github.com/apache/spark/pull/46521


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2707088340

   Get your point, and let me respond in the voting mail


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2998868245

   Merged into master for Apache Spark 4.1.0. Thanks @pan3793 @yaooqinn @wangyum and @dongjoon-hyun 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-48231][BUILD] Remove unused CodeHaus Jackson dependencies [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #46521:
URL: https://github.com/apache/spark/pull/46521#issuecomment-2998582023

   thanks @dongjoon-hyun.
   
   cc @wangyum @LuciferYang, this is ready for review. also cc @Madhukar525722, who asked this before.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org