korlov42 commented on a change in pull request #672:
URL: https://github.com/apache/ignite-3/pull/672#discussion_r840405546
##########
File path:
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelInputEx.java
##########
@@ -38,8 +40,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.
Review comment:
I believe the index name doesn't matter here
##########
File path:
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelInputEx.java
##########
@@ -38,8 +40,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.
Review comment:
```suggestion
* Returns an index by its id.
```
##########
File path:
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/AbstractIndexScan.java
##########
@@ -88,8 +98,8 @@ protected RelWriter explainTerms0(RelWriter pw) {
* Get index name.
* TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859
*/
Review comment:
could you please fix javadoc as well?
##########
File path:
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/schema/SqlSchemaManagerImpl.java
##########
@@ -106,6 +111,18 @@ public IgniteTable tableById(UUID id) {
return table;
}
+ /** {@inheritDoc} */
+ @Override
+ public InternalSortedIndex indexById(UUID id, String idxName) {
+ InternalSortedIndex idx = indexManager.getIndexById(id);
+
+ if (idx == null) {
+ throw new IndexNotFoundException(idxName, id);
Review comment:
This doesn't seem right to pass an `idxName` param only for case when
index is not found. Probably, there should be one more constructor of
`IndexNotFoundException` accepting the `Id` only
##########
File path:
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/externalize/RelJsonReaderTest.java
##########
@@ -71,4 +76,45 @@ void fromJson() {
assertThat(node.getTable().unwrap(IgniteTable.class),
is(igniteTableMock));
Mockito.verify(schemaMock).tableById(tableId);
}
+
+ @Test
+ void fromJsonIdxScan() {
+ UUID indexId = UUID.randomUUID();
+ UUID tableId = UUID.randomUUID();
+
+ IgniteTable igniteTableMock = mock(IgniteTable.class);
+ InternalSortedIndex igniteIdxMock = mock(InternalSortedIndex.class);
+ when(igniteTableMock.getStatistic()).thenReturn(new Statistic() {});
+ when(mock(RelInputEx.class).getIndexById(any(),
any())).thenReturn(igniteIdxMock);
+
when(igniteTableMock.getRowType(any())).thenReturn(mock(RelDataType.class));
+
when(igniteTableMock.unwrap(InternalIgniteTable.class)).thenReturn(mock(InternalIgniteTable.class));
+
+ SqlSchemaManager schemaMock = mock(SqlSchemaManager.class);
+
+ when(schemaMock.tableById(tableId)).thenReturn(igniteTableMock);
+ when(schemaMock.indexById(any(UUID.class),
any())).thenReturn(igniteIdxMock);
+
+ String json = ""
+ + "{\n"
+ + " \"rels\" : [ {\n"
+ + " \"id\" : \"0\",\n"
+ + " \"relOp\" : \"IgniteIndexScan\",\n"
+ + " \"index\" : \"idx0\",\n"
+ + " \"table\" : [\"PUBLIC\", \"TEST\"],\n"
+ + " \"indexId\" : \"" + indexId + "\",\n"
+ + " \"tableId\" : \"" + tableId + "\",\n"
+ + " \"inputs\" : [ ]\n"
+ + " } ]\n"
+ + "}";
+
+ RelNode node = RelJsonReader.fromJson(schemaMock, json);
+
+ assertThat(node, isA(IgniteIndexScan.class));
+ assertThat(node.getTable(), notNullValue());
+ assertThat(node.getTable().unwrap(IgniteTable.class),
is(igniteTableMock));
+ assertThat(IgniteTestUtils.getFieldValue(node,
AbstractIndexScan.class, "index"), notNullValue());
+
+ Mockito.verify(schemaMock).tableById(tableId);
+ Mockito.verify(schemaMock).indexById(any(UUID.class), any());
Review comment:
```suggestion
Mockito.verify(schemaMock).indexById(indexId, any());
```
--
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]
