Github user rdblue commented on the pull request:
https://github.com/apache/spark/pull/13236#issuecomment-221063988
@yhuai, Hive uses shims to be compatible with Hadoop 1 and Hadoop 2. I
think it would be better to use the existing mechanism in Hive to deal with
this.
I know that that this didn't use the copy constructor before, but that was
a bug. That had a few flaws, including the fact that properties set in the
hadoopConf didn't show up in the HiveConf. While this version accounts for
missing properties, the behavior of those properties will not be correct
according to the normal way Hadoop/Hive work: final properties can be
accidentally overwritten by this implementation. The addDefaultResource problem
is just another way this causes Spark's behavior to diverge from what users
expect and there may be others that I didn't think of.
I think the ideal situation is to continue using the copy constructor to
avoid unknown behavior differences, but at a minimum I think this needs to
correctly copy final properties to the new HiveConf. The class loader situation
sounds fine as you describe it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
