asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ahmed Eldawy (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: [ASTERIXDB-1371][FUN][AQL][SQL] Add standard geometry data t...
Date Sat, 03 Feb 2018 01:16:42 GMT
Ahmed Eldawy has posted comments on this change.

Change subject: [ASTERIXDB-1371][FUN][AQL][SQL] Add standard geometry data type and functions
......................................................................


Patch Set 7:

(3 comments)

I didn't test its memory consumption but my understanding is that it will create as many Java
objects as the number of records in the input. I understand how bad it is but this is the
price of using the existing Esri library. It is not really optimized for streaming data from
a binary stream where it reuses the same object over and over. I don't see anyway around it
without changing the Esri library itself. However, we can probably have our own implementation
for the easy functions (e.g., XMin and XMax) which does not create any objects and processes
it from the binary stream. But we will still rely on Esri for the complex functions (e.g.,
Union and Intersect for multipolygon.

 > (4 comments)
 > 
 > Has this been benchmarked for memory consumption? I see a lot of
 > places that look like to me, they will consume memory in proportion
 > to the data perhaps...

https://asterix-gerrit.ics.uci.edu/#/c/2056/6/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/serde/AGeometrySerializerDeserializer.java
File asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/serde/AGeometrySerializerDeserializer.java:

PS6, Line 49:             OGCGeometry geometry = OGCGeometry.createFromOGCStructure(
            :                     OperatorImportFromWkb.local().executeOGC(0, buffer, null),
SpatialReference.create(4326));
            :             return new AGeometry(geometry);
> They should both be static final variables then, the numbers.
Done


https://asterix-gerrit.ics.uci.edu/#/c/2056/6/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/STUnionAggregateFunction.java
File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/STUnionAggregateFunction.java:

PS6, Line 66: 
            :     @
> That should definitely be a static final variable then.
Done


Line 104:     }
> Maybe use this UnsupportedItemTypeException?
Done


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2056
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9cddeffea42e85469b6fc38f361bd98e64025289
Gerrit-PatchSet: 7
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ahmed Eldawy <eldawy@cs.umn.edu>
Gerrit-Reviewer: Ahmed Eldawy <aseldawy@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Xikui Wang <xkkwww@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message