korlov42 commented on a change in pull request #672:
URL: https://github.com/apache/ignite-3/pull/672#discussion_r838339996
##########
File path:
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/AbstractIndexScan.java
##########
@@ -51,6 +57,7 @@
protected AbstractIndexScan(RelInput input) {
super(input);
idxName = input.getString("index");
Review comment:
Looks like `idxName` is actually used to request the index from the
table. Thus, I believe it would be better to keep `IgniteIndex` within this
scan node.
##########
File path:
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteIndex.java
##########
@@ -35,7 +37,9 @@
private final String idxName;
- private final InternalSortedIndex idx;
+ private final UUID id;
+
+ private @Nullable final InternalSortedIndex idx;
Review comment:
delegating index should never be null
##########
File path:
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJsonReader.java
##########
@@ -75,8 +80,8 @@
* FromJson.
* TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859
*/
- public static <T extends RelNode> T fromJson(SqlSchemaManager
schemaManager, String json) {
- RelJsonReader reader = new RelJsonReader(schemaManager);
+ public static <T extends RelNode> T fromJson(SqlSchemaManager
schemaManager, IndexManager idxManager, String json) {
+ RelJsonReader reader = new RelJsonReader(schemaManager, idxManager);
Review comment:
I believe that `schemaManager` should be enough to work with every
schema object.
##########
File path:
modules/index-api/src/main/java/org/apache/ignite/internal/idx/IndexManager.java
##########
@@ -102,4 +102,12 @@ InternalSortedIndex createIndex(
* @throws NodeStoppingException If an implementation stopped before the
method was invoked.
*/
CompletableFuture<InternalSortedIndex> indexAsync(String idxCanonicalName);
+
+ /**
+ * Check index presence with current id.
+ *
+ * @param id Index id.
+ * @return {@code true} if found.
+ */
+ boolean getIndexById(UUID id);
Review comment:
```suggestion
InternalSortedIndex index(UUID id);
```
##########
File path:
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/IgniteIndex.java
##########
@@ -44,26 +48,27 @@
* TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859
*/
public IgniteIndex(RelCollation collation, String name,
InternalIgniteTable tbl) {
Review comment:
It's better to drop this constructor and the next one because someone
could accidentally use it in production code.
##########
File path:
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelInputEx.java
##########
@@ -38,8 +39,17 @@
/**
* Returns table by its id.
*
- * @param tag Tag under which id is serialised.
+ * @param tag Tag under which id is serialized.
* @return A table with given id.
*/
RelOptTable getTableById(String tag);
+
+ /**
+ * Check that appropriate index is available.
+ *
+ * @param tag Tag represents id serialization.
+ * @param idxName Index name.
+ * @throws IndexNotFoundException If not found.
+ */
+ void checkIndexById(String tag, String idxName);
Review comment:
why do we need a `idxName` param here?
--
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]
