[GitHub] [carbondata] ShreelekhyaG opened a new pull request #4118: [WIP] Fix case sensitive issues and input validation for Geo values.

[GitBox]

Indhumathi27 commented on a change in pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#discussion_r620864904



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/CarbonSourceStrategy.scala
##########
@@ -62,10 +62,53 @@ private[sql] object CarbonSourceStrategy extends SparkStrategy {
         } catch {
           case _: CarbonPhysicalPlanException => Nil
         }
+      case Project(projectList, _: OneRowRelation) if validateUdf(projectList) => Nil
       case _ => Nil
     }
   }
 
+  private def validateUdf(projects: Seq[NamedExpression]): Boolean = {
+    projects foreach {
+      case alias: Alias if alias.child.isInstanceOf[Expression] =>
+        alias.child match {
+          case Cast(s: ScalaUDF, _, _) => validateGeoUtilUDFs(s)
+          case s: ScalaUDF => validateGeoUtilUDFs(s)
+          case _ =>
+        }
+    }
+    true
+  }
+
+  def validateGeoUtilUDFs(s: ScalaUDF): Boolean = {
+    var geoUdf = false
+    if (s.function.isInstanceOf[ToRangeListUDF]) {
+      GeoHashUtils.validateUDFInputValue(s.children.head.toString(), "polygon", "String")

Review comment:
       instead of specifying datatype, can use from s.inputTypes(0).typeName




--
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:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#issuecomment-827326345


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5270/
   


--
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:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#issuecomment-827326403


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3525/
   


--
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:
[hidden email]

[GitBox]

ShreelekhyaG commented on a change in pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#discussion_r620911442



##########
File path: integration/spark/src/main/scala/org/apache/carbondata/geo/GeoUtilUDFs.scala
##########
@@ -33,34 +33,51 @@ object GeoUtilUDFs {
   }
 }
 
-class GeoIdToGridXyUDF extends (Long => Array[Int]) with Serializable {
-  override def apply(geoId: Long): Array[Int] = {
+class GeoIdToGridXyUDF extends (java.lang.Long => Array[Int]) with Serializable {
+  override def apply(geoId: java.lang.Long): Array[Int] = {
+    GeoHashUtils.validateUDFInputValue(geoId, "geoId", "Long")
     GeoHashUtils.geoID2ColRow(geoId)
   }
 }
 
-class GeoIdToLatLngUDF extends ((Long, Double, Int) => Array[Double]) with Serializable {
-  override def apply(geoId: Long, oriLatitude: Double, gridSize: Int): Array[Double] = {
+class GeoIdToLatLngUDF
+  extends ((java.lang.Long, java.lang.Double, java.lang.Integer) => Array[Double]) with
+    Serializable {
+  override def apply(geoId: java.lang.Long, oriLatitude: java.lang.Double,
+      gridSize: java.lang.Integer): Array[Double] = {
+    GeoHashUtils.validateUDFInputValue(geoId, "geoId", "Long")
+    GeoHashUtils.validateUDFInputValue(oriLatitude, "oriLatitude", "Double")
+    GeoHashUtils.validateUDFInputValue(gridSize, "gridSize", "Integer")
     GeoHashUtils.geoID2LatLng(geoId, oriLatitude, gridSize)
   }
 }
 
-class LatLngToGeoIdUDF extends ((Long, Long, Double, Int) => Long) with Serializable {
-  override def apply(latitude: Long, longitude: Long, oriLatitude: Double, gridSize: Int): Long = {
+class LatLngToGeoIdUDF extends ((java.lang.Long, java.lang.Long,
+  java.lang.Double, java.lang.Integer) => Long) with Serializable {
+  override def apply(latitude: java.lang.Long, longitude: java.lang.Long,
+      oriLatitude: java.lang.Double, gridSize: java.lang.Integer): Long = {
+    GeoHashUtils.validateUDFInputValue(latitude, "latitude", "Long")
+    GeoHashUtils.validateUDFInputValue(longitude, "longitude", "Long")
+    GeoHashUtils.validateUDFInputValue(oriLatitude, "oriLatitude", "Double")
+    GeoHashUtils.validateUDFInputValue(gridSize, "gridSize", "Integer")
     GeoHashUtils.lonLat2GeoID(longitude, latitude, oriLatitude, gridSize)
   }
 }
 
-class ToUpperLayerGeoIdUDF extends (Long => Long) with Serializable {
-  override def apply(geoId: Long): Long = {
+class ToUpperLayerGeoIdUDF extends (java.lang.Long => Long) with Serializable {
+  override def apply(geoId: java.lang.Long): Long = {
+    GeoHashUtils.validateUDFInputValue(geoId, "geoId", "Long")
     GeoHashUtils.convertToUpperLayerGeoId(geoId)
   }
 }
 
-class ToRangeListUDF extends ((String, Double, Int) => mutable.Buffer[Array[Long]])
-  with Serializable {
-  override def apply(polygon: String, oriLatitude: Double,
-                     gridSize: Int): mutable.Buffer[Array[Long]] = {
+class ToRangeListUDF extends ((java.lang.String, java.lang.Double, java.lang.Integer) =>
+  mutable.Buffer[Array[Long]]) with Serializable {
+  override def apply(polygon: java.lang.String, oriLatitude: java.lang.Double,
+      gridSize: java.lang.Integer): mutable.Buffer[Array[Long]] = {
+    GeoHashUtils.validateUDFInputValue(polygon, "polygon", "String")

Review comment:
       Done

##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/CarbonSourceStrategy.scala
##########
@@ -62,10 +62,53 @@ private[sql] object CarbonSourceStrategy extends SparkStrategy {
         } catch {
           case _: CarbonPhysicalPlanException => Nil
         }
+      case Project(projectList, _: OneRowRelation) if validateUdf(projectList) => Nil
       case _ => Nil
     }
   }
 
+  private def validateUdf(projects: Seq[NamedExpression]): Boolean = {
+    projects foreach {
+      case alias: Alias if alias.child.isInstanceOf[Expression] =>
+        alias.child match {
+          case Cast(s: ScalaUDF, _, _) => validateGeoUtilUDFs(s)
+          case s: ScalaUDF => validateGeoUtilUDFs(s)
+          case _ =>
+        }
+    }
+    true
+  }
+
+  def validateGeoUtilUDFs(s: ScalaUDF): Boolean = {
+    var geoUdf = false

Review comment:
       Done




--
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:
[hidden email]

[GitBox]

ShreelekhyaG commented on a change in pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#discussion_r620911580



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/CarbonSourceStrategy.scala
##########
@@ -62,10 +62,53 @@ private[sql] object CarbonSourceStrategy extends SparkStrategy {
         } catch {
           case _: CarbonPhysicalPlanException => Nil
         }
+      case Project(projectList, _: OneRowRelation) if validateUdf(projectList) => Nil
       case _ => Nil
     }
   }
 
+  private def validateUdf(projects: Seq[NamedExpression]): Boolean = {
+    projects foreach {
+      case alias: Alias if alias.child.isInstanceOf[Expression] =>
+        alias.child match {
+          case Cast(s: ScalaUDF, _, _) => validateGeoUtilUDFs(s)
+          case s: ScalaUDF => validateGeoUtilUDFs(s)
+          case _ =>
+        }
+    }
+    true
+  }
+
+  def validateGeoUtilUDFs(s: ScalaUDF): Boolean = {
+    var geoUdf = false
+    if (s.function.isInstanceOf[ToRangeListUDF]) {
+      GeoHashUtils.validateUDFInputValue(s.children.head.toString(), "polygon", "String")

Review comment:
       ok, done




--
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:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#issuecomment-827412287


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5273/
   


--
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:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#issuecomment-827414341


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3528/
   


--
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:
[hidden email]

[GitBox]

Indhumathi27 commented on pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#issuecomment-827436458






--
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:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#issuecomment-827496342


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5278/
   


--
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:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#issuecomment-827496922


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3533/
   


--
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:
[hidden email]

[GitBox]

ShreelekhyaG commented on pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#issuecomment-827500501


   retest this please


--
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:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#issuecomment-827554970






--
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:
[hidden email]

[GitBox]

asfgit closed pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118


   


--
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:
[hidden email]