Stamatis, Vladimir, thanks for your comments. The empty constructor Stamatis is suggesting would probably be enough, but what Vladimir is suggesting would also improve on the annotation front (in particular, it's easier to annotate just the constructor rather than each field in the whole hierarchy).
I am using Jackson from time to time but I did not know about that feature, I will dig more and try to improve the current implementation I have. Would you mind writing a small example of what you have in mind? Googling the topic a bit more, I also found ConstructorParameters <https://docs.oracle.com/javase/9/docs/api/javax/management/ConstructorParameters.html> annotation (Java7+) which might help: > @ConstructorParameters({"aid", "name", "birthPlace", "books"}) public Author(int aid, String name, Place birthPlace, List<Book> books) { > this.aid = aid; > this.name = name; > this.birthPlace = birthPlace; > this.books = books; > } Regarding "newExpression", any test making use of a JavaRowType is affected (e.g., ReflectiveSchemaTest.testSelectWithFieldAccessOnSecondLevelRecordType <https://github.com/apache/calcite/blob/c475124ef8c61eb398ee119f27dfe21a14eca32f/core/src/test/java/org/apache/calcite/test/ReflectiveSchemaTest.java#L336>), but the dependency on the field order does not come from the test itself, but it's tied to the way expressions representing rows are built for enumerable tables. The aforementioned test runs the following query: "select au.\"birthPlace\".\"coords\".\"latitude\" as lat\n" + "from \"bookstore\".\"authors\" au\n" While building rows for "Authors" table (backed up by a Java object), JavaRowFormat.record() <https://github.com/apache/calcite/blob/c475124ef8c61eb398ee119f27dfe21a14eca32f/core/src/main/java/org/apache/calcite/adapter/enumerable/JavaRowFormat.java#L61> is invoked, and it builds an expression of this kind, which invokes Authors's constructor with the appropriate parameters: new org.apache.calcite.test.BookstoreSchema.Author( > row.aid, > row.name, > row.birthPlace, > org.apache.calcite.linq4j.Linq4j.asEnumerable(row.books).select(new > org.apache.calcite.linq4j.function.Function1() { > public java.util.List > apply(org.apache.calcite.test.BookstoreSchema.Book o) { > return (java.util.List) > org.apache.calcite.runtime.FlatLists.of(o.title, o.publishYear, o.pages); > } > public Object apply(Object o) { > return apply( > (org.apache.calcite.test.BookstoreSchema.Book) o); > } > } In this case, since my "default" ordering was different from (sorting alphabetically) the "declaration order" used by the JVM, I had to provide the same ordering via the CalciteField annotation, in order to still match the constructor: public static class Author { @CalciteField(order = 1) public final int aid; @CalciteField(order = 2) public final String name; @CalciteField(order = 3) public final Place birthPlace; @CalciteField(order = 4) @org.apache.calcite.adapter.java.Array(component = Book.class) public final List<Book> books; public Author(int aid, String name, Place birthPlace, List<Book> books) { this.aid = aid; this.name = name; this.birthPlace = birthPlace; this.books = books; } } If you start from master, and you swap any two parameters in Author (either in the field declaration, or in the constructor), all tests related to Author will fail since the constructor invocation part won't compile due to signature mismatch (the same apply to any other such class used in tests). For instance, I moved "name" field as the last declared field in Author, and below you can see that the constructor invocation expects it as last parameter: java.sql.SQLException: Error while executing SQL "select > au."birthPlace"."coords"."latitude" as lat > from "bookstore"."authors" au > ": Error while compiling generated Java code: > public org.apache.calcite.linq4j.Enumerable bind(final > org.apache.calcite.DataContext root) { > final org.apache.calcite.linq4j.Enumerable _inputEnumerable = > org.apache.calcite.linq4j.Linq4j.asEnumerable(((org.apache.calcite.test.BookstoreSchema) > ((org.apache.calcite.adapter.java.ReflectiveSchema) > root.getRootSchema().getSubSchema("bookstore").unwrap(org.apache.calcite.adapter.java.ReflectiveSchema.class)).getTarget()).authors).select(new > org.apache.calcite.linq4j.function.Function1() { > public org.apache.calcite.test.BookstoreSchema.Author > apply(org.apache.calcite.test.BookstoreSchema.Author row) { > return new org.apache.calcite.test.BookstoreSchema.Author( > row.aid, > row.birthPlace, > > org.apache.calcite.linq4j.Linq4j.asEnumerable(row.books).select(new > org.apache.calcite.linq4j.function.Function1() { > public java.util.List > apply(org.apache.calcite.test.BookstoreSchema.Book o) { > return org.apache.calcite.runtime.FlatLists.of(o.title, > o.publishYear, o.pages); > } > public Object apply(Object o) { > return apply( > (org.apache.calcite.test.BookstoreSchema.Book) o); > } > } > ).toList(), > row.name); <----- should be the second param > } > public Object apply(Object row) { > return apply( > (org.apache.calcite.test.BookstoreSchema.Author) row); > } > } > ); [...] > It's NewExpression.accept() (NewExpression.java#L63 <https://github.com/apache/calcite/blob/c475124ef8c61eb398ee119f27dfe21a14eca32f/linq4j/src/main/java/org/apache/calcite/linq4j/tree/NewExpression.java#L63>) generating that string, which indirectly receives the parameter order from JavaTypeFactoryImpl.createStructType(Class type) <https://github.com/apache/calcite/blob/c475124ef8c61eb398ee119f27dfe21a14eca32f/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java#L78>, which it was using Class.getFields(), so in the end the output of that method has always been matching the declaration order, which is also the order used for the constructor of all the Java objects used in Calcite codebase (that's why in the diff you can see I had to put annotations in all of them to keep the same expected test results). Best regards, Alessandro On Tue, 9 Feb 2021 at 11:35, Vladimir Sitnikov <sitnikov.vladi...@gmail.com> wrote: > Alessandro, thanks for pushing this. > > Frankly speaking, I don't like @CalciteField(order=42) approach. It looks > like annotating the constructor parameters makes more sense, and it is > more flexible. > A similar case is covered in Jackson with "parameter names" module: > > https://github.com/FasterXML/jackson-modules-java8/tree/master/parameter-names > Just in case, Java8+ can include parameter names to the reflection info in > case user supplies `-parameters` option for the compiler. > > >NewExpression > > It might indeed be a too low-level API for that. > Could you please highlight which test relies on the order of constructor > arguments? > > By the way, SQL structs rely on the order of the fields rather than field > names. > So it might be ok to rely on the order of the constructor arguments. > > Vladimir >
