denis-chudov commented on code in PR #1296:
URL: https://github.com/apache/ignite-3/pull/1296#discussion_r1011972649
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaManager.java:
##########
@@ -301,14 +275,7 @@ private CompletableFuture<SchemaDescriptor>
tableSchema(UUID tblId, String table
CompletableFuture<SchemaDescriptor> fut = new CompletableFuture<>();
if (checkSchemaVersion(tblId, schemaVer)) {
- SchemaDescriptor desc = searchSchemaByVersion(tblId, schemaVer);
-
- if (desc == null) {
- return getSchemaDescriptor(schemaVer, tblCfg);
- } else {
- fut.complete(desc);
- return fut;
- }
+ return getSchemaDescriptor(schemaVer, tblCfg);
Review Comment:
Seems that now any call of `tableSchema` method causes request to
metastorage. Assuming this is justified (this method is only used for closure
that is called only if the schema was not found in schema cache, see
`SchemaRegistryImpl#schema(int)`), doesn't it deserve mentioning in javadoc?
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaUtils.java:
##########
@@ -45,38 +45,53 @@ public static SchemaDescriptor prepareSchemaDescriptor(int
schemaVer, TableView
* Prepares column mapper.
*
* @param oldDesc Old schema descriptor.
- * @param oldTblColumns Old columns configuration.
* @param newDesc New schema descriptor.
- * @param newTblColumns New columns configuration.
* @return Column mapper.
*/
public static ColumnMapper columnMapper(
SchemaDescriptor oldDesc,
- NamedListView<? extends ColumnView> oldTblColumns,
- SchemaDescriptor newDesc,
- NamedListView<? extends ColumnView> newTblColumns
+ SchemaDescriptor newDesc
) {
- ColumnMapper mapper = null;
+ // Rename not supported from standard, thus only deletion.
+ if (newDesc.keyColumns().length() != oldDesc.keyColumns().length()) {
+ Optional<Column> droppedKeyCol =
Arrays.stream(oldDesc.keyColumns().columns())
+ .filter(c ->
!newDesc.column(c.schemaIndex()).name().equals(c.name()))
+ .findAny();
+
Review Comment:
I barely know anything related to schema logic, but the old algorithm of
selecting `droppedKeyCol` seems more solid to me, are you sure the new one is
better? What guarantees that the key columns in new schema will have the same
indexes as corresponding ones in old schema?
--
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]
