asolimando opened a new pull request #2488: URL: https://github.com/apache/calcite/pull/2488
### Summary: The PR follows the approach suggested in the ML discussion [discussion](https://mail-archives.apache.org/mod_mbox/calcite-dev/202102.mbox/%3CCAFtOckKUyaqBHe3q66svZWyX77Zwj0Kc_D4%2B49m75SthuPj%2BGQ%40mail.gmail.com%3E) from several months ago. In a nutshell, the output of "Class.getFields()" is JVM implementation-specific without any guarantees on the field ordering, the PR aims at providing a deterministic and stable ordering. `ReflectiveSchema` is where users can define a schema starting from a Java class. Now the schema descriptor has an extra parameter for specifying the sought parameter ordering, among the followings: - JVM, // same as Class.getFields() - ALPHABETICAL, // alphabetical order - ALPHABETICAL_AND_HIERARCHY, // alphabetical order but inherited fields come first - CONSTRUCTOR, // order is taken from the constructor matching the highest number of fields - EXPLICIT, // field ordering is explicitly provided - EXPLICIT_TOLERANT // field ordering is explicitly provided, but can be incomplete `CONSTRUCTOR` is being used as default ordering (it generally matches the order of the declared fields, which is what all the JVM implementations I could test use, thus ensuring the least number of diff in existing unit tests), and `ALPHABETICAL_AND_HIERARCHY` as a fallback ordering for classes lacking the name of the parameters in their constructor(s). ### Walkthrough of the commits in the PR (from "older" to "newer"): 1. implementation of the replacement for `Class.getFields()` for `ReflectiveSchema` and the auxiliary methods in `Types` helper class 2. added unit tests covering the aforementioned changes 3. replacing `Class.getFields()` and `Class.getDeclaredFields()` anywhere in the code where the "order" of such fields matters 4. added `-parameters` compilation parameter to enrich Calcite's classes with parameters names for their constructors (more details are in the aforementioned ML discussion) and constructor-based field ordering as default 5. adapted few existing unit tests due to changes in field order coming from (4) ### How the PR was tested: Several unit tests have been added, covering all the new or changed features, tests are passing locally and in CI. -- 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]
