juliuszsompolski commented on issue #25721: [WIP][SPARK-29018][SQL] Implement 
Spark Thrift Server with it's own code base on PROTOCOL_VERSION_V9
URL: https://github.com/apache/spark/pull/25721#issuecomment-545579648
 
 
   > > 2. on top of that, all the Thriftserver code that is just translated 
from Java to Scala without changes - it has a new package now, so it will 
remain unused.
   
   > Translate all code seems too heavy. And we build it with protocol v11. we 
can't direct apply to v1.2.1/v2.3.5.
   > I prefer to build the framework based on protocol v11 such as common class 
and process like Operation.class, SessionManager.class, then fill in the details
   
   I am confused. It seems that in this PR you already did translation from 
Java to Scala of all Hive Thriftserver code? I assume it was mostly somehow 
autogenerated?
   A lot of these classes don't really change much from Hive to Spark, except 
for mechanical translation from Java to Scala, renaming package names, removing 
dependence on some Hive objects. Do I see correctly?
   I would commit these in a separate PR, to separate "mechanical" changes from 
places that were actually rewritten.
   
   I am also not sure whether we should translate those from Java to Scala at 
all. Maybe we should keep these in Java code, and only implement the Spark 
specific stuff in scala, removing Java Hive stuff that is not needed anymore. 
So e.g.
   - Keep CLIService.java, ThriftCLIService.java, ThriftHttpServlet.java, ... - 
all things that don't really get modified by Spark in Java
   - Do Spark specific implementation in scala, and remove the no longer needed 
Java thriftserver impl. E.g. (Spark)ExecuteStatementOperation.scala and remove 
ExecuteStatementOperation.java; (Spark)OperationManager.scala and remove 
OperationManager.java etc. etc.
   OR
   Translate all these Java files to scala, like is done in this PR?
   
   I think I would keep them in Java to avoid potential errors in translation, 
and also to see easier if we want to port any future Hive changes to them.
   @gatorsmile @rxin - what do you think?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

Reply via email to