QL: Extend NodeSubclass to read classes from jars #50866
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As the test classes are spread across more than one project, the Gradle classpath contains not just folders but also jars. This commit allows the test class to explore the archive content and load matching classes from said source.
costin
added
>refactoring
:Analytics/SQL
|
Pinging @elastic/es-search (:Search/SQL) |
bpintea
reviewed
Jan 14, 2020
| String name = je.getName(); | ||
| if (name.endsWith(".class")) { | ||
| String className = name.substring(0, name.length() - ".class".length()).replace("/", "."); | ||
| maybeLoadClass(clazz, className, root + "!/" + name, results); |
There was a problem hiding this comment.
Is !/ intended as a visual marker in the path? The location is later only used for the message in the Exception, so I guess so, but curious about it.
There was a problem hiding this comment.
No, it's a special marker inside the URL to indicate the path inside the archive:
jar:/my/file.jar!/org/my/package/clazz.class
bpintea
reviewed
Jan 14, 2020
| /** | ||
| * Load classes from predefined packages (hack to limit the scope) and if they match the hierarchy, add them to the cache | ||
| */ | ||
| private static <T> void maybeLoadClass(Class<T> clazz, String className, String location, List<Class<? extends T>> results) |
There was a problem hiding this comment.
Depending on the name, the class might be loaded or not hence the 'maybe' versus attempt which suggests the loading will happen always (but it might fail).
bpintea
approved these changes
Jan 14, 2020
matriv
approved these changes
Jan 14, 2020
| /** | ||
| * Load classes from predefined packages (hack to limit the scope) and if they match the hierarchy, add them to the cache | ||
| */ | ||
| private static <T> void maybeLoadClass(Class<T> clazz, String className, String location, List<Class<? extends T>> results) |
There was a problem hiding this comment.
Shouldn't the results be a Set instead of List?
costin
force-pushed
the
ql/fix-test-jar-classpath
branch
from
3be7fbd
to
9ef7c2b
Compare
SivagurunathanV
pushed a commit
to SivagurunathanV/elasticsearch
that referenced
this pull request
Jan 23, 2020
As the test classes are spread across more than one project, the Gradle classpath contains not just folders but also jars. This commit allows the test class to explore the archive content and load matching classes from said source.
costin
added a commit
that referenced
this pull request
Jan 27, 2020
* Introduce reusable QL plugin for SQL and EQL (#50815) Extract reusable functionality from SQL into its own dedicated project QL. Implemented as a plugin, it provides common components across SQL and the upcoming EQL. While this commit is fairly large, for the most part it's just a big file move from sql package to the newly introduced ql. (cherry picked from commit ec1ac0d) * SQL: Fix incomplete registration of geo NamedWritables (cherry picked from commit e295763) * QL: Extend NodeSubclass to read classes from jars (#50866) As the test classes are spread across more than one project, the Gradle classpath contains not just folders but also jars. This commit allows the test class to explore the archive content and load matching classes from said source. (cherry picked from commit 25ad749) * QL: Remove implicit conversion inside Literal (#50962) Literal constructor makes an implicit conversion for each value given which turns out has some subtle side-effects. Improve MathProcessors to preserve numeric type where possible Fix bug on issue compatibility between date and intervals Preserve the source when folding inside the Optimizer (cherry picked from commit 9b73e22) * QL: Refactor DataType for pluggability (#51328) Change DataType from enum to class Break DataType enums into QL (default) and SQL types Make data type conversion pluggable so that new types can be introduced As part of the process: - static type conversion in QL package (such as Literal) has been removed - several utility classes have been broken into base (QL) and extended (SQL) parts based on type awareness - operators (+,-,/,*) are - due to extensibility, serialization of arithmetic operation has been slightly changed and pushed down to the operator executor itself (cherry picked from commit aebda81) * Compilation fixes for 7.x
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As the test classes are spread across more than one project, the Gradle
classpath contains not just folders but also jars.
This commit allows the test class to explore the archive content and
load matching classes from said source.
With the fix in place, two test classes affected by this (around 600 tests) are now enabled.