>From Suryaa Charan Shivakumar <[email protected]>: Attention is currently required from: Ian Maxon.
Suryaa Charan Shivakumar has posted comments on this change by Suryaa Charan Shivakumar. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20968?usp=email ) Change subject: [ASTERIXDB-3542] Added Coordinate Reference System support at Metadata and ST Transform function ...................................................................... Patch Set 8: (8 comments) Patchset: PS8: Made the changes in patchset 8, thank you File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/handlers/CRSStatementHandler.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20968/comment/cdd9c8a2_4b1ea526?usp=email : PS7, Line 115: // Validate WKT only for new CRS entries > why's that? WKT parsing via CRS.fromWKT(...) happens after the existing != null early-returns. So if the user runs CREATE COORDINATE REFERENCE SYSTEM IF NOT EXISTS 4326 NAME "WGS 84" AS "<anything>" and SRID 4326 already exists, the WKT string is never parsed, the statement commits as a no-op and returns. The comment can be removed if its confusing File asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/GeoFunctionTypeInferers.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20968/comment/0a6b875e_40c6476c?usp=email : PS7, Line 69: public static final class STTransformTypeInferer implements IFunctionTypeInferer { : @Override : public void infer(ILogicalExpression expr, IFunctionDescriptor fd, IVariableTypeEnvironment context, : CompilerProperties compilerProps) throws AlgebricksException { : throw new CompilationException(ErrorCode.COMPILATION_ERROR, expr.getSourceLocation(), : "ST_Transform requires a default dataverse to be set (USE <dataverse>)"); : } : : @Override : public void infer(ILogicalExpression expr, IFunctionDescriptor fd, IVariableTypeEnvironment context, : CompilerProperties compilerProps, Namespace defaultNamespace) throws AlgebricksException { : AbstractFunctionCallExpression fce = (AbstractFunctionCallExpression) expr; > i think this is not the right way to go about this. there is always a > namespace, implied or not. […] Maybe an optimizer rule (that can be reused later) resolves the two CRS WKTs from metadata and stashes them on the ST_Transform call via setOpaqueParameters, which the type inferer later just reads and forwards to the descriptor? File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataCache.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20968/comment/97b29c68_50dedfb8?usp=email : PS7, Line 98: crsMap > maybe just 'crs' here Yes I will make it in line with FullText- configMetadataEntity. crs was used in methods so I wanted to make this crsMap to distinguish/code readability. File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20968/comment/1aca2e2b_f1909d4a?usp=email : PS7, Line 3258: public static ITupleReference createIntTuple(int value) { : try { : ArrayTupleBuilder tupleBuilder = new ArrayTupleBuilder(1); : ISerializerDeserializer<AInt32> int32Serde = : SerializerDeserializerProvider.INSTANCE.getSerializerDeserializer(BuiltinType.AINT32); : AMutableInt32 aInt32 = new AMutableInt32(value); : tupleBuilder.addField(int32Serde, aInt32); : ArrayTupleReference tuple = new ArrayTupleReference(); : tuple.reset(tupleBuilder.getFieldEndOffsets(), tupleBuilder.getByteArray()); : return tuple; : } catch (HyracksDataException e) { : // This should never happen : throw new IllegalStateException("Failed to create search tuple", e); : } : } > this doesn't seem to be used Sorry Dead code from previous patchset, when it was keyed with SRID alone. Will be removed. File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/CoordinateReferenceSystemEntity.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20968/comment/de2cc367_00941326?usp=email : PS7, Line 1: /* > this is kind of a nit but i would just rename this to > CoordinateReferenceSystem or CRS to be consist […] Yes will do this, the file name is already long enough. File asterixdb/pom.xml: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20968/comment/19b1eb13_a2f61d5f?usp=email : PS7, Line 131: > nit: blank line Will be removed. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20968/comment/b9b5883f_bd0f47b9?usp=email : PS7, Line 188: <exclude>**/*.wkt</exclude> > i didn't see any . […] This is dead code, previously I had WKT files as part of test resources. It was meant for bulk load. Since that is pushed to a future improvement, this can be removed. In terms of comments in WKT, I believe the standard doesn't support it. Parsers are also not very friendly with it, CRS.fromWKT method which we use doesn't avoid comments while parsing, we will have to write additional code to clean it up, that's why I added this in the first place. Will be removed in the next patchset. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20968?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: Ia6e37080a581292744ddc9020b214926412c16ac Gerrit-Change-Number: 20968 Gerrit-PatchSet: 8 Gerrit-Owner: Suryaa Charan Shivakumar <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-CC: Ian Maxon <[email protected]> Gerrit-Attention: Ian Maxon <[email protected]> Gerrit-Comment-Date: Thu, 16 Apr 2026 22:35:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ian Maxon <[email protected]>
