jiayuasu commented on code in PR #1001:
URL: https://github.com/apache/sedona/pull/1001#discussion_r1316548721


##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/raster/RasterBandAccessors.scala:
##########
@@ -45,6 +52,34 @@ case class RS_SummaryStats(inputExpressions: 
Seq[Expression]) extends InferredEx
   }
 }
 
+case class RS_Band(inputExpressions: Seq[Expression]) extends Expression with 
SerdeAware with CodegenFallback {

Review Comment:
   Why not use the inferredexpression?



##########
common/src/main/java/org/apache/sedona/common/raster/RasterBandAccessors.java:
##########
@@ -134,6 +135,25 @@ public static double[] getSummaryStats(GridCoverage2D 
raster) {
 //        return getSummaryStats(raster, 1, excludeNoDataValue);
 //    }
 
+    public static GridCoverage2D getBand(GridCoverage2D rasterGeom, int[] 
bands) throws FactoryException {
+        double[] metadata = RasterAccessors.metadata(rasterGeom);
+        GridCoverage2D resultRaster = 
RasterConstructors.makeEmptyRaster(bands.length, (int) metadata[2], (int) 
metadata[3],
+                metadata[0], metadata[1], metadata[4], metadata[5], 
metadata[6], metadata[7], (int) metadata[8]);
+        double[] curBandData;
+        Double noDataValue;
+        for (int curBand: bands) {
+            RasterUtils.ensureBand(rasterGeom, curBand);
+            curBandData = MapAlgebra.bandAsArray(rasterGeom, curBand);
+            noDataValue = RasterBandAccessors.getBandNoDataValue(rasterGeom, 
curBand);
+            if (noDataValue != null) {
+                resultRaster = MapAlgebra.addBandFromArray(resultRaster, 
curBandData, curBand, noDataValue);

Review Comment:
   You should not directly use `addBandFromArray`. Each function call will copy 
and create a new raster, which is very slow. You should implement a version of 
`copyRasterAndReplaceBand` or borrow some logic from that to set pixel values 
of all bands in one go.



##########
spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala:
##########
@@ -100,6 +100,10 @@ object InferrableType {
     new InferrableType[Array[Byte]] {}
   implicit val longArrayInstance: InferrableType[Array[java.lang.Long]] =
     new InferrableType[Array[java.lang.Long]] {}
+  implicit val intArrayInstance: InferrableType[Array[Int]] =
+    new InferrableType[Array[Int]] {}
+  implicit val javaIntArrayInstance: InferrableType[Array[java.lang.Integer]] =

Review Comment:
   Why do you need both `Int` and `java.lang.Integer`?



-- 
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]

Reply via email to