You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/11/23 13:19:37 UTC

[GitHub] [ignite-3] sashapolo opened a new pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

sashapolo opened a new pull request #468:
URL: https://github.com/apache/ignite-3/pull/468


   https://issues.apache.org/jira/browse/IGNITE-15885


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766743157



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       Unfortunately, no. `BinaryRow` serialization places columns differently than specified in the schema. This means that some column, that is missing in the prefix, can actually be the first bytes of the serialized row. What I'm trying to say is that some prefix column values might not serialize as a byte prefix of the serialized row =( 

##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       I thought that I couldn't, but actually I can, let me try

##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       Unfortunately no. The problem is that it will be impossible to reuse the `BinaryRowComparator`, because of `BinaryRow` serialization works: a serialized prefix and a serialized full row can have a different internal column order, therefore it will be impossible to de-serialize the prefix using the full row schema

##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       Unfortunately no. The problem is that it will be impossible to reuse the `BinaryRowComparator`, because of how `BinaryRow` serialization works: a serialized prefix and a serialized full row can have a different internal column order, therefore it will be impossible to de-serialize the prefix using the full row schema

##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/ByteBufferRow.java
##########
@@ -137,7 +137,11 @@ public double readDouble(int off) {
     /** {@inheritDoc} */
     @Override
     public String readString(int off, int len) {
-        return new String(buf.array(), off, len, StandardCharsets.UTF_8);
+        if (buf.hasArray()) {
+            return new String(buf.array(), off, len, StandardCharsets.UTF_8);

Review comment:
       no, it won't

##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;

Review comment:
       https://issues.apache.org/jira/browse/IGNITE-16105




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r763172076



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;

Review comment:
       Isn't this a bit too preemptive? This field isn't even used in index-based accesses.




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768743095



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       IndexRow may contain value columns.
   This may be useful for index-only scans, otherwise we will need to go for a DataRow just to get indexed value.
   However, we could change this later, when needed.




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] SammyVimes commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
SammyVimes commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768533602



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/TableStorage.java
##########
@@ -52,10 +52,32 @@
      *
      * @param partId Partition id.
      * @throws IllegalArgumentException If partition id is out of bounds.
-     * @throws StorageException If an error has occurred during the partition destruction.
+     * @throws StorageException         If an error has occurred during the partition destruction.
      */
     void dropPartition(int partId) throws StorageException;
 
+    /**
+     * Creates or returns an already created Sorted Index with the given name.
+     *
+     * <p>A prerequisite for calling this method is to have the index already configured under the same name in the Table Configuration

Review comment:
       <p> should have a closing tag

##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/basic/BinarySearchRow.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.basic;
+
+import java.nio.ByteBuffer;
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.storage.SearchRow;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Adapter that converts a {@link BinaryRow} into a {@link SearchRow}.
+ */
+public class BinarySearchRow implements SearchRow {
+    /** Key array. */
+    private final byte[] key;
+
+    /**
+     * The constructor.
+     *
+     * @param row The search row.
+     */
+    public BinarySearchRow(BinaryRow row) {
+        // TODO asch IGNITE-15934 can reuse thread local byte buffer
+        key = new byte[row.keySlice().capacity()];
+
+        row.keySlice().get(key);
+    }
+
+    @Override
+    public byte @NotNull [] keyBytes() {
+        return key;
+    }
+
+    @Override
+    public @NotNull ByteBuffer key() {

Review comment:
       Is this our new codestyle? Oh god, please no........

##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
##########
@@ -87,53 +73,42 @@
     /** Data region for the table. */
     private final RocksDbDataRegion dataRegion;
 
-    /** Comparators factory for indexes. */
-    private final BiFunction<TableView, String, Comparator<ByteBuffer>> indexComparatorFactory;
-
     /** List of closeable resources to close on {@link #stop()}. Better than having a field for each one of them. */
     private final List<AutoCloseable> autoCloseables = new ArrayList<>();
 
-    /** Rocks DB instance itself. */
+    /** Rocks DB instance. */
     private RocksDB db;
 
     /** CF handle for meta information. */
     @SuppressWarnings("unused")
-    private ColumnFamilyHandle metaCfHandle;
+    private ColumnFamily metaCf;

Review comment:
       Why is it unused tho?

##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/BinaryIndexRowDeserializer.java
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor.ColumnDescriptor;
+
+/**
+ * {@link IndexRowDeserializer} implementation that uses {@link BinaryRow} infrastructure for deserialization purposes.
+ */
+class BinaryIndexRowDeserializer implements IndexRowDeserializer {
+    private final SortedIndexDescriptor descriptor;
+
+    BinaryIndexRowDeserializer(SortedIndexDescriptor descriptor) {
+        this.descriptor = descriptor;
+    }
+
+    @Override
+    public Object[] indexedColumnValues(IndexRow indexRow) {
+        var row = new Row(descriptor.asSchemaDescriptor(), new ByteBufferRow(indexRow.rowBytes()));
+
+        return descriptor.indexRowColumns().stream()

Review comment:
       I wonder how hot this code will be. Should it use streams?

##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/BinaryRowComparator.java
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import static java.util.Comparator.comparing;
+import static java.util.Comparator.comparingDouble;
+import static java.util.Comparator.comparingInt;
+import static java.util.Comparator.comparingLong;
+import static org.apache.ignite.internal.storage.rocksdb.index.ComparatorUtils.comparingNull;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.Arrays;
+import java.util.Comparator;
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.NativeTypeSpec;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.rocksdb.AbstractComparator;
+import org.rocksdb.ComparatorOptions;
+
+/**
+ * Naive RocksDB comparator implementation that fully de-serializes the passed data and compares the index columns one after the other.
+ */
+public class BinaryRowComparator extends AbstractComparator {
+    /**
+     * Actual comparator implementation.
+     */
+    private final Comparator<ByteBuffer> innerComparator;
+
+    /**
+     * Options needed for resource management.
+     */
+    private final ComparatorOptions options;
+
+    /**
+     * Creates a RocksDB comparator for a Sorted Index identified by the given descriptor.
+     */
+    public BinaryRowComparator(SortedIndexDescriptor descriptor) {
+        this(descriptor, new ComparatorOptions());
+    }
+
+    /**
+     * Internal constructor for capturing the {@code options} parameter for resource management purposes.
+     */
+    private BinaryRowComparator(SortedIndexDescriptor descriptor, ComparatorOptions options) {
+        super(options);
+
+        innerComparator = comparing(
+                byteBuffer -> new ByteBufferRow(byteBuffer.order(ByteOrder.LITTLE_ENDIAN)),
+                binaryRowComparator(descriptor)
+        );
+
+        this.options = options;
+    }
+
+    /**
+     * Creates a comparator for comparing two {@link BinaryRow}s by converting them into {@link Row}s.
+     */
+    private static Comparator<BinaryRow> binaryRowComparator(SortedIndexDescriptor descriptor) {
+        return comparing(
+                binaryRow -> new Row(descriptor.asSchemaDescriptor(), binaryRow),
+                rowComparator(descriptor)
+        );
+    }
+
+    /**
+     * Creates a comparator that compares two {@link Row}s by comparing individual columns.
+     */
+    private static Comparator<Row> rowComparator(SortedIndexDescriptor descriptor) {
+        return descriptor.indexRowColumns().stream()
+                .map(columnDescriptor -> {
+                    Column column = columnDescriptor.column();
+
+                    Comparator<Row> columnComparator = columnComparator(column);
+
+                    if (columnDescriptor.nullable()) {
+                        columnComparator = comparingNull(
+                                row -> row.hasNullValue(column.schemaIndex(), column.type().spec()) ? null : row,
+                                columnComparator
+                        );
+                    }
+
+                    return columnDescriptor.asc() ? columnComparator : columnComparator.reversed();
+                })
+                .reduce(Comparator::thenComparing)
+                .orElseThrow();
+    }
+
+    /**
+     * Creates a comparator for comparing table columns.
+     */
+    private static Comparator<Row> columnComparator(Column column) {
+        int schemaIndex = column.schemaIndex();
+
+        NativeTypeSpec typeSpec = column.type().spec();
+
+        switch (typeSpec) {
+            case INT8:
+                return (row1, row2) -> {
+                    byte value1 = row1.byteValue(schemaIndex);
+                    byte value2 = row2.byteValue(schemaIndex);
+
+                    return Byte.compare(value1, value2);
+                };
+
+            case INT16:
+                return (row1, row2) -> {
+                    short value1 = row1.shortValue(schemaIndex);
+                    short value2 = row2.shortValue(schemaIndex);
+
+                    return Short.compare(value1, value2);
+                };
+
+            case INT32:
+                return comparingInt(row -> row.intValue(schemaIndex));
+
+            case INT64:
+                return comparingLong(row -> row.longValue(schemaIndex));
+
+            case FLOAT:
+                return (row1, row2) -> {
+                    float value1 = row1.floatValue(schemaIndex);
+                    float value2 = row2.floatValue(schemaIndex);
+
+                    return Float.compare(value1, value2);
+                };
+
+            case DOUBLE:
+                return comparingDouble(row -> row.doubleValue(schemaIndex));
+
+            case BYTES:
+                return comparing(row -> row.bytesValue(schemaIndex), Arrays::compare);
+
+            case BITMASK:
+                return comparing(row -> row.bitmaskValue(schemaIndex).toLongArray(), Arrays::compare);
+
+            // all other types implement Comparable
+            case DECIMAL:
+            case UUID:
+            case STRING:
+            case NUMBER:
+            case TIMESTAMP:
+            case DATE:
+            case TIME:
+            case DATETIME:
+                return comparing(row -> (Comparable) typeSpec.objectValue(row, schemaIndex));
+
+            default:
+                throw new IllegalArgumentException(String.format(
+                        "Unsupported column schema for creating a sorted index. Column name: %s, column type: %s",
+                        column.name(), column.type()
+                ));
+        }
+    }
+
+    @Override

Review comment:
       inheritDoc here and after

##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/TableStorage.java
##########
@@ -52,10 +52,32 @@
      *
      * @param partId Partition id.
      * @throws IllegalArgumentException If partition id is out of bounds.
-     * @throws StorageException If an error has occurred during the partition destruction.
+     * @throws StorageException         If an error has occurred during the partition destruction.
      */
     void dropPartition(int partId) throws StorageException;
 
+    /**
+     * Creates or returns an already created Sorted Index with the given name.
+     *
+     * <p>A prerequisite for calling this method is to have the index already configured under the same name in the Table Configuration
+     * (see {@link #configuration()}).
+     *
+     * @param indexName Index name.
+     * @return Sorted Index storage.
+     * @throws StorageException if no index has been configured under the given name or it has been configured incorrectly (e.g. it was
+     *                          configured as a Hash Index).
+     */
+    SortedIndexStorage getOrCreateSortedIndex(String indexName);
+
+    /**
+     * Destroys the index under the given name and all data in it.
+     *
+     * <p>This method is a no-op if the index under the given name does not exist.

Review comment:
       <p> should have a closing tag

##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.tostring.S;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        private final boolean nullable;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn, boolean nullable) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+            this.nullable = nullable;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        /**
+         * Returns {@code true} if this column can contain null values or {@code false} otherwise.
+         */
+        public boolean nullable() {
+            return nullable;
+        }
+
+        @Override
+        public String toString() {
+            return S.toString(this);
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream();
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition
+                .map(columnName -> {
+                    ColumnView columnView = tableConfig.columns().get(columnName);
+
+                    assert columnView != null : "Incorrect index column configuration. " + columnName + " column does not exist";
+
+                    return columnView;
+                })
+                .collect(toList());
+
+        schemaDescriptor = createSchemaDescriptor(indexKeyColumnViews);
+
+        columns = Arrays.stream(schemaDescriptor.keyColumns().columns())
+                .sorted(Comparator.comparingInt(Column::columnOrder))
+                .map(column -> {
+                    IndexColumnView columnView = indexColumns.get(column.name());
+
+                    // if the index config does not contain this column - it's a column from the Primary Key
+                    boolean indexedColumn = columnView != null;
+
+                    // PK columns are always sorted in ascending order
+                    boolean asc = !indexedColumn || columnView.asc();
+
+                    return new ColumnDescriptor(column, asc, indexedColumn, column.nullable());
+                })
+                .collect(toUnmodifiableList());
+    }
+
+    /**
+     * Creates a {@link SchemaDescriptor} from a list of index key columns.
+     */
+    private static SchemaDescriptor createSchemaDescriptor(List<ColumnView> indexKeyColumnViews) {
+        Column[] keyColumns = new Column[indexKeyColumnViews.size()];
+
+        for (int i = 0; i < indexKeyColumnViews.size(); ++i) {
+            ColumnView columnView = indexKeyColumnViews.get(i);
+
+            ColumnDefinition columnDefinition = SchemaConfigurationConverter.convert(columnView);
+
+            keyColumns[i] = SchemaDescriptorConverter.convert(i, columnDefinition);
+        }
+
+        return new SchemaDescriptor(0, keyColumns, new Column[0]);
+    }
+
+    /**
+     * Returns this index' name.
+     */
+    public String name() {
+        return name;
+    }
+
+    /**
+     * Returns the Column Descriptors that comprise a row of this index (indexed columns + primary key columns).
+     */
+    public List<ColumnDescriptor> indexRowColumns() {
+        return columns;
+    }
+
+    /**
+     * Converts this Descriptor into an equivalent {@link SchemaDescriptor}.
+     *
+     * <p>The resulting {@code SchemaDescriptor} will have empty {@link SchemaDescriptor#valueColumns()} and its

Review comment:
       <p> should have a closing tag

##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/IndexRow.java
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.PartitionStorage;
+import org.apache.ignite.internal.storage.SearchRow;
+
+/**
+ * Represents an Index Row - a set of indexed columns and Primary Key columns (for key uniqueness).
+ */
+public interface IndexRow {
+    /**
+     * Returns the serialized presentation of this row as a byte array.
+     *
+     * @return Serialized byte array value.
+     */
+    byte[] rowBytes();
+
+    /**
+     * Returns the Primary Key that is a part of this row.
+     *
+     * <p>This is a convenience method for easier extraction of the Primary Key to use it for accessing the {@link PartitionStorage}.

Review comment:
       <p> should have a closing tag

##########
File path: modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorageTest.java
##########
@@ -0,0 +1,497 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+import static org.apache.ignite.internal.schema.SchemaTestUtils.generateRandomValue;
+import static org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter.convert;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.randomBytes;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
+import static org.apache.ignite.schema.SchemaBuilders.column;
+import static org.apache.ignite.schema.SchemaBuilders.tableBuilder;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Predicate;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.schemas.store.DataRegionConfiguration;
+import org.apache.ignite.configuration.schemas.table.HashIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.SortedIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.TableConfiguration;
+import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.engine.DataRegion;
+import org.apache.ignite.internal.storage.engine.TableStorage;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor.ColumnDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageEngine;
+import org.apache.ignite.internal.testframework.VariableSource;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.internal.util.Cursor;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.lang.IgniteLogger;
+import org.apache.ignite.schema.SchemaBuilders;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+import org.apache.ignite.schema.definition.ColumnType;
+import org.apache.ignite.schema.definition.TableDefinition;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder.SortedIndexColumnBuilder;
+import org.apache.ignite.schema.definition.index.ColumnarIndexDefinition;
+import org.apache.ignite.schema.definition.index.HashIndexDefinition;
+import org.apache.ignite.schema.definition.index.SortedIndexDefinition;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.RepeatedTest;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+
+/**
+ * Test class for the {@link RocksDbSortedIndexStorage}.
+ */
+@ExtendWith(WorkDirectoryExtension.class)
+@ExtendWith(ConfigurationExtension.class)
+public class RocksDbSortedIndexStorageTest {
+    private static final IgniteLogger log = IgniteLogger.forClass(RocksDbSortedIndexStorageTest.class);
+
+    private static final Random random = new Random();

Review comment:
       I think random should be seeded, so we can have repeatable tests. Also, I'm not sure that @RepeatedTest annotation will provide any information about the definitions. Maybe these tests should be parameterized too?

##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexRowFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        assert row.rowBytes().length > 0;
+        assert row.primaryKey().keyBytes().length > 0;
+
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();
+
+        return new RocksIteratorAdapter<>(iter) {
+            @Nullable
+            private PrefixComparator lowerBoundComparator = new PrefixComparator(descriptor, lowerBound);
+
+            private final PrefixComparator upperBoundComparator = new PrefixComparator(descriptor, upperBound);
+
+            @Override
+            public boolean hasNext() {
+                while (super.hasNext()) {
+                    var row = new ByteBufferRow(it.key());
+
+                    if (lowerBoundComparator != null) {
+                        // if lower comparator is not null, then the lower bound has not yet been reached
+                        if (lowerBoundComparator.compare(row) < 0) {
+                            it.next();
+                        } else {
+                            // once the lower bound is reached, we no longer need to check it
+                            lowerBoundComparator = null;
+
+                            return upperBoundComparator.compare(row) <= 0;
+                        }
+                    } else {
+                        return upperBoundComparator.compare(row) <= 0;
+                    }

Review comment:
       ```suggestion
           if (lowerBoundComparator != null) {
               // if lower comparator is not null, then the lower bound has not yet been reached
               if (lowerBoundComparator.compare(row) < 0) {
                   it.next();
                   continue;
               } else {
                   // once the lower bound is reached, we no longer need to check it
                   lowerBoundComparator = null;
               }
           }
           return upperBoundComparator.compare(row) <= 0;
   ```




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768757446



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.tostring.S;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        private final boolean nullable;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn, boolean nullable) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+            this.nullable = nullable;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        /**
+         * Returns {@code true} if this column can contain null values or {@code false} otherwise.
+         */
+        public boolean nullable() {
+            return nullable;
+        }
+
+        @Override
+        public String toString() {
+            return S.toString(this);
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream();
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition
+                .map(columnName -> {
+                    ColumnView columnView = tableConfig.columns().get(columnName);
+
+                    assert columnView != null : "Incorrect index column configuration. " + columnName + " column does not exist";
+
+                    return columnView;
+                })
+                .collect(toList());
+
+        schemaDescriptor = createSchemaDescriptor(indexKeyColumnViews);
+
+        columns = Arrays.stream(schemaDescriptor.keyColumns().columns())
+                .sorted(Comparator.comparingInt(Column::columnOrder))

Review comment:
       Will this be the same?
   ```suggestion
           columns = indexKeyColumnViews.stream()
   ```




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r762977618



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;

Review comment:
       I think List-wrapper is useless, and can be replaced with ColumnDescriptor[].
   Descriptors will be used on hot path, and array offers clear O(1) access complexity.




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r764773944



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       > Agree with separate class for range args (I'd call it "IndexSearchRow" for now), because it's structure may differs.
   
   What's wrong with the current name?




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766743552



##########
File path: modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorageTest.java
##########
@@ -0,0 +1,468 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+import static org.apache.ignite.internal.schema.SchemaTestUtils.generateRandomValue;
+import static org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter.convert;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
+import static org.apache.ignite.schema.SchemaBuilders.column;
+import static org.apache.ignite.schema.SchemaBuilders.tableBuilder;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Predicate;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.schemas.store.DataRegionConfiguration;
+import org.apache.ignite.configuration.schemas.table.HashIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.PartialIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.SortedIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.TableConfiguration;
+import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.engine.DataRegion;
+import org.apache.ignite.internal.storage.engine.TableStorage;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor.ColumnDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageEngine;
+import org.apache.ignite.internal.testframework.VariableSource;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.internal.util.Cursor;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.lang.IgniteLogger;
+import org.apache.ignite.schema.SchemaBuilders;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+import org.apache.ignite.schema.definition.ColumnType;
+import org.apache.ignite.schema.definition.TableDefinition;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder.SortedIndexColumnBuilder;
+import org.apache.ignite.schema.definition.index.ColumnarIndexDefinition;
+import org.apache.ignite.schema.definition.index.HashIndexDefinition;
+import org.apache.ignite.schema.definition.index.SortedIndexDefinition;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.RepeatedTest;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+
+/**
+ * Test class for the {@link RocksDbSortedIndexStorage}.
+ */
+@ExtendWith(WorkDirectoryExtension.class)
+@ExtendWith(ConfigurationExtension.class)
+public class RocksDbSortedIndexStorageTest {
+    private static final IgniteLogger log = IgniteLogger.forClass(RocksDbSortedIndexStorageTest.class);
+
+    private static final Random random = new Random();
+
+    /**
+     * Definitions of all supported column types.
+     */
+    private static final List<ColumnDefinition> ALL_TYPES_COLUMN_DEFINITIONS = allTypesColumnDefinitions();
+
+    @InjectConfiguration(polymorphicExtensions = {
+            HashIndexConfigurationSchema.class,

Review comment:
       I also need another schema as well, but ok, I'll remove the redundant one




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768680431



##########
File path: modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorageTest.java
##########
@@ -0,0 +1,497 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+import static org.apache.ignite.internal.schema.SchemaTestUtils.generateRandomValue;
+import static org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter.convert;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.randomBytes;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
+import static org.apache.ignite.schema.SchemaBuilders.column;
+import static org.apache.ignite.schema.SchemaBuilders.tableBuilder;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Predicate;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.schemas.store.DataRegionConfiguration;
+import org.apache.ignite.configuration.schemas.table.HashIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.SortedIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.TableConfiguration;
+import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.engine.DataRegion;
+import org.apache.ignite.internal.storage.engine.TableStorage;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor.ColumnDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageEngine;
+import org.apache.ignite.internal.testframework.VariableSource;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.internal.util.Cursor;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.lang.IgniteLogger;
+import org.apache.ignite.schema.SchemaBuilders;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+import org.apache.ignite.schema.definition.ColumnType;
+import org.apache.ignite.schema.definition.TableDefinition;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder.SortedIndexColumnBuilder;
+import org.apache.ignite.schema.definition.index.ColumnarIndexDefinition;
+import org.apache.ignite.schema.definition.index.HashIndexDefinition;
+import org.apache.ignite.schema.definition.index.SortedIndexDefinition;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.RepeatedTest;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+
+/**
+ * Test class for the {@link RocksDbSortedIndexStorage}.
+ */
+@ExtendWith(WorkDirectoryExtension.class)
+@ExtendWith(ConfigurationExtension.class)
+public class RocksDbSortedIndexStorageTest {
+    private static final IgniteLogger log = IgniteLogger.forClass(RocksDbSortedIndexStorageTest.class);
+
+    private static final Random random = new Random();

Review comment:
       > I think random should be seeded
   
   fixed




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766920984



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       Unfortunately no. The problem is that it will be impossible to reuse the `BinaryRowComparator`, because of how `BinaryRow` serialization works: a serialized prefix and a serialized full row can have a different internal column order, therefore it will be impossible to de-serialize the prefix using the full row 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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r770401462



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.tostring.S;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        private final boolean nullable;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn, boolean nullable) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+            this.nullable = nullable;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        /**
+         * Returns {@code true} if this column can contain null values or {@code false} otherwise.
+         */
+        public boolean nullable() {
+            return nullable;
+        }
+
+        @Override
+        public String toString() {
+            return S.toString(this);
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream();
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition
+                .map(columnName -> {
+                    ColumnView columnView = tableConfig.columns().get(columnName);
+
+                    assert columnView != null : "Incorrect index column configuration. " + columnName + " column does not exist";
+
+                    return columnView;
+                })
+                .collect(toList());
+
+        schemaDescriptor = createSchemaDescriptor(indexKeyColumnViews);
+
+        columns = Arrays.stream(schemaDescriptor.keyColumns().columns())
+                .sorted(Comparator.comparingInt(Column::columnOrder))

Review comment:
       But you use columnOrder().
   Its an order the column are in the index descriptor... seems, you just restore the indexKeyColumnViews order 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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r770417696



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       I've added an API that extracts index column values from the `IndexRow`, I hope that addresses your comment




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766745047



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        @Override
+        public String toString() {
+            return "ColumnDescriptor{"
+                    + "name=" + column.name()
+                    + ", type=" + column.type().spec()
+                    + ", asc=" + asc
+                    + ", indexedColumn=" + indexedColumn
+                    + '}';
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream()

Review comment:
       ok, I got it, nice catch




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768626364



##########
File path: modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorageTest.java
##########
@@ -0,0 +1,497 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+import static org.apache.ignite.internal.schema.SchemaTestUtils.generateRandomValue;
+import static org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter.convert;
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.randomBytes;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
+import static org.apache.ignite.schema.SchemaBuilders.column;
+import static org.apache.ignite.schema.SchemaBuilders.tableBuilder;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Predicate;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.schemas.store.DataRegionConfiguration;
+import org.apache.ignite.configuration.schemas.table.HashIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.SortedIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.TableConfiguration;
+import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.engine.DataRegion;
+import org.apache.ignite.internal.storage.engine.TableStorage;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor.ColumnDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageEngine;
+import org.apache.ignite.internal.testframework.VariableSource;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.internal.util.Cursor;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.lang.IgniteLogger;
+import org.apache.ignite.schema.SchemaBuilders;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+import org.apache.ignite.schema.definition.ColumnType;
+import org.apache.ignite.schema.definition.TableDefinition;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder.SortedIndexColumnBuilder;
+import org.apache.ignite.schema.definition.index.ColumnarIndexDefinition;
+import org.apache.ignite.schema.definition.index.HashIndexDefinition;
+import org.apache.ignite.schema.definition.index.SortedIndexDefinition;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.RepeatedTest;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+
+/**
+ * Test class for the {@link RocksDbSortedIndexStorage}.
+ */
+@ExtendWith(WorkDirectoryExtension.class)
+@ExtendWith(ConfigurationExtension.class)
+public class RocksDbSortedIndexStorageTest {
+    private static final IgniteLogger log = IgniteLogger.forClass(RocksDbSortedIndexStorageTest.class);
+
+    private static final Random random = new Random();

Review comment:
       > Also, I'm not sure that @RepeatedTest annotation will provide any information about the definitions.
   
   That's why I've added `log.info` messages for each iteration




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768627243



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/basic/BinarySearchRow.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.basic;
+
+import java.nio.ByteBuffer;
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.storage.SearchRow;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Adapter that converts a {@link BinaryRow} into a {@link SearchRow}.
+ */
+public class BinarySearchRow implements SearchRow {
+    /** Key array. */
+    private final byte[] key;
+
+    /**
+     * The constructor.
+     *
+     * @param row The search row.
+     */
+    public BinarySearchRow(BinaryRow row) {
+        // TODO asch IGNITE-15934 can reuse thread local byte buffer
+        key = new byte[row.keySlice().capacity()];
+
+        row.keySlice().get(key);
+    }
+
+    @Override
+    public byte @NotNull [] keyBytes() {
+        return key;
+    }
+
+    @Override
+    public @NotNull ByteBuffer key() {

Review comment:
       I don't know, it's how IDEA has formatted it...




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768625816



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
##########
@@ -87,53 +73,42 @@
     /** Data region for the table. */
     private final RocksDbDataRegion dataRegion;
 
-    /** Comparators factory for indexes. */
-    private final BiFunction<TableView, String, Comparator<ByteBuffer>> indexComparatorFactory;
-
     /** List of closeable resources to close on {@link #stop()}. Better than having a field for each one of them. */
     private final List<AutoCloseable> autoCloseables = new ArrayList<>();
 
-    /** Rocks DB instance itself. */
+    /** Rocks DB instance. */
     private RocksDB db;
 
     /** CF handle for meta information. */
     @SuppressWarnings("unused")
-    private ColumnFamilyHandle metaCfHandle;
+    private ColumnFamily metaCf;

Review comment:
       This field was introduced by @ibessonov and he states that it is reserved for future purposes... Or do you mean that we should use it somehow?




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768773248



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.tostring.S;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        private final boolean nullable;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn, boolean nullable) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+            this.nullable = nullable;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        /**
+         * Returns {@code true} if this column can contain null values or {@code false} otherwise.
+         */
+        public boolean nullable() {
+            return nullable;
+        }
+
+        @Override
+        public String toString() {
+            return S.toString(this);
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream();
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition
+                .map(columnName -> {
+                    ColumnView columnView = tableConfig.columns().get(columnName);
+
+                    assert columnView != null : "Incorrect index column configuration. " + columnName + " column does not exist";
+
+                    return columnView;
+                })
+                .collect(toList());
+
+        schemaDescriptor = createSchemaDescriptor(indexKeyColumnViews);
+
+        columns = Arrays.stream(schemaDescriptor.keyColumns().columns())
+                .sorted(Comparator.comparingInt(Column::columnOrder))

Review comment:
       Unfortunately, no. `columns` returned from `indexKeyColumnViews` don't have a `schemaIndex` =(




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766920984



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       Unfortunately no. The problem is that it will be impossible to reuse the `BinaryRowComparator`, because of `BinaryRow` serialization works: a serialized prefix and a serialized full row can have a different internal column order, therefore it will be impossible to de-serialize the prefix using the full row 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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766919022



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       I thought that I couldn't, but actually I can, let me try




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] tledkov-gridgain commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r764917141



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       @AMashenkov i'm OK with proposal while we don't use value of the Rocks DB for sorted storage.
   We have to change the interface in the future.

##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        SortedIndexView indexConfig = (SortedIndexView) tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = indexConfig.columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream()
+                .map(indexColumns::get)
+                .map(IndexColumnView::name);
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition

Review comment:
       Should we use ordered stream here to preserve order of the columns? 




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768743095



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       IndexRow may contain value columns.
   This may be useful for index-only scans, otherwise we will need to go for a DataRow just to get indexed value.
   However, we could change this later.




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768747522



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        SortedIndexView indexConfig = (SortedIndexView) tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = indexConfig.columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream()
+                .map(indexColumns::get)
+                .map(IndexColumnView::name);
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition

Review comment:
       Concat method javadoc says
   ```
      * The resulting stream is ordered if both
      * of the input streams are ordered, and parallel if either of the input
      * streams is parallel
   ```




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] SammyVimes merged pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
SammyVimes merged pull request #468:
URL: https://github.com/apache/ignite-3/pull/468


   


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r765919194



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       > Just to be consistent with Data storage interface.
   
   Sorry, but I do not agree. `SearchRow` represents the Primary Key, while `IndexKeyPrefix` is a prefix of indexed columns. I'm ok with renaming it to `IndexRowPrefix` to be consistent with the `IndexRow` class 




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] ibessonov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
ibessonov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766688539



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/ByteBufferRow.java
##########
@@ -137,7 +137,11 @@ public double readDouble(int off) {
     /** {@inheritDoc} */
     @Override
     public String readString(int off, int len) {
-        return new String(buf.array(), off, len, StandardCharsets.UTF_8);
+        if (buf.hasArray()) {
+            return new String(buf.array(), off, len, StandardCharsets.UTF_8);

Review comment:
       Just curious, will it work for sliced byte buffer?

##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        @Override
+        public String toString() {
+            return "ColumnDescriptor{"
+                    + "name=" + column.name()
+                    + ", type=" + column.type().spec()
+                    + ", asc=" + asc
+                    + ", indexedColumn=" + indexedColumn
+                    + '}';
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream()

Review comment:
       I'm a little bit confused, but maybe what you need here is indexColumns.namedListKeys().stream(). Maybe I'm wrong, please check it.

##########
File path: modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorageTest.java
##########
@@ -0,0 +1,468 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+import static org.apache.ignite.internal.schema.SchemaTestUtils.generateRandomValue;
+import static org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter.convert;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
+import static org.apache.ignite.schema.SchemaBuilders.column;
+import static org.apache.ignite.schema.SchemaBuilders.tableBuilder;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Predicate;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.schemas.store.DataRegionConfiguration;
+import org.apache.ignite.configuration.schemas.table.HashIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.PartialIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.SortedIndexConfigurationSchema;
+import org.apache.ignite.configuration.schemas.table.TableConfiguration;
+import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.engine.DataRegion;
+import org.apache.ignite.internal.storage.engine.TableStorage;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor.ColumnDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageEngine;
+import org.apache.ignite.internal.testframework.VariableSource;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.internal.util.Cursor;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.lang.IgniteLogger;
+import org.apache.ignite.schema.SchemaBuilders;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+import org.apache.ignite.schema.definition.ColumnType;
+import org.apache.ignite.schema.definition.TableDefinition;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder;
+import org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder.SortedIndexColumnBuilder;
+import org.apache.ignite.schema.definition.index.ColumnarIndexDefinition;
+import org.apache.ignite.schema.definition.index.HashIndexDefinition;
+import org.apache.ignite.schema.definition.index.SortedIndexDefinition;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.RepeatedTest;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+
+/**
+ * Test class for the {@link RocksDbSortedIndexStorage}.
+ */
+@ExtendWith(WorkDirectoryExtension.class)
+@ExtendWith(ConfigurationExtension.class)
+public class RocksDbSortedIndexStorageTest {
+    private static final IgniteLogger log = IgniteLogger.forClass(RocksDbSortedIndexStorageTest.class);
+
+    private static final Random random = new Random();
+
+    /**
+     * Definitions of all supported column types.
+     */
+    private static final List<ColumnDefinition> ALL_TYPES_COLUMN_DEFINITIONS = allTypesColumnDefinitions();
+
+    @InjectConfiguration(polymorphicExtensions = {
+            HashIndexConfigurationSchema.class,

Review comment:
       Having only "SortedIndexConfigurationSchema" is enough here, you can remove the others

##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        @Override
+        public String toString() {
+            return "ColumnDescriptor{"

Review comment:
       Maybe you should use one of S.toString methods

##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       Can you call something like ".seek(lowerBound.bytes())" instead of starting from the beginning of the index? Would be more efficient if it's possible




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766741503



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        @Override
+        public String toString() {
+            return "ColumnDescriptor{"
+                    + "name=" + column.name()
+                    + ", type=" + column.type().spec()
+                    + ", asc=" + asc
+                    + ", indexedColumn=" + indexedColumn
+                    + '}';
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream()

Review comment:
       Sorry, I don't understand. Isn't that what is written in the code?




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#issuecomment-986603431


   I've see 3 similar interfaces. SearchRow, DataRow, IndexKey.
   PartitionStorage methods contract looks easy to understand,  
   ```
   DataRow read(SearchRow)
   void write(DataRow)
   ```
   however, SortedIndexStorage barely readable
   ```
   SearchRow get(IndexKey key)
   void put(IndexKey key, SearchRow value);
   ```
   
   Maybe it will be better to rename SearchRow -> KeyRow, IndexKey -> SearchRow.
   


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r762980430



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;

Review comment:
       I'm ok if SchemaDescriptor will be used underneath as temporary solution,
   but let's avoid confusion with versioned schema descriptor for tables, and introduce e.g. IndexDescriptor as early as possible.
   Guess Index row will have different structure than Data row.
   




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r763172345



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;

Review comment:
       sure, I will later create a ticket




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r767146302



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;

Review comment:
       https://issues.apache.org/jira/browse/IGNITE-16105




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766929590



##########
File path: modules/schema/src/main/java/org/apache/ignite/internal/schema/ByteBufferRow.java
##########
@@ -137,7 +137,11 @@ public double readDouble(int off) {
     /** {@inheritDoc} */
     @Override
     public String readString(int off, int len) {
-        return new String(buf.array(), off, len, StandardCharsets.UTF_8);
+        if (buf.hasArray()) {
+            return new String(buf.array(), off, len, StandardCharsets.UTF_8);

Review comment:
       no, it won't




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r770415057



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.tostring.S;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        private final boolean nullable;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn, boolean nullable) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+            this.nullable = nullable;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        /**
+         * Returns {@code true} if this column can contain null values or {@code false} otherwise.
+         */
+        public boolean nullable() {
+            return nullable;
+        }
+
+        @Override
+        public String toString() {
+            return S.toString(this);
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream();
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition
+                .map(columnName -> {
+                    ColumnView columnView = tableConfig.columns().get(columnName);
+
+                    assert columnView != null : "Incorrect index column configuration. " + columnName + " column does not exist";
+
+                    return columnView;
+                })
+                .collect(toList());
+
+        schemaDescriptor = createSchemaDescriptor(indexKeyColumnViews);
+
+        columns = Arrays.stream(schemaDescriptor.keyColumns().columns())
+                .sorted(Comparator.comparingInt(Column::columnOrder))

Review comment:
       yes, but I later need the correct `schemaIndex` inside the `BinaryRowComparator` for example. So, the only problem is that `indexKeyColumnViews` do not contain fully initialized `Column` objects, that are later needed. I could extract this columns from the `SchemaDescriptor`, but I found it strange that columns inside `SchemaDescriptor` and `columns` will not be interchangeable




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r762980430



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;

Review comment:
       I'm ok if SchemaDescriptor will be used underneath as temporary solution,
   but let's avoid confusion with versioned schema descriptor for tables, and introduce e.g. IndexDescriptor as early as possible.
   
   Guess Index row will have different structure comparing Data row.




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r764767716



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       > Returning 'boolean' may be useful. Does't it costs anything or easy to do?
   
   It is relatively easy to implement but it will cost a lookup: RocksDB does not return any booleans, so we will have to manually check. I don't think it is worth doing now....




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r764859475



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       > What's wrong with the current name?
   Just to be consistent with Data storage interface.




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766743157



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       Unfortunately, no. `BinaryRow` serialization places columns differently than specified in the schema. This means that some column, that is missing in the prefix, can actually be the first bytes of the serialized row. What I'm trying to say is that some prefix column values might not serialize as a byte prefix of the serialized row =( 




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r764755028



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       Can we have smth like this.
   ```suggestion
       /**
        * Adds the given index row to the index.
        */
       void put(IndexRow row);
   
       /**
        * Removes the given row from the index.
        */
       void remove(IndexRow row);
   
       /**
        * Returns a range of index rows between the lower bound (inclusive) and the upper bound (inclusive).
        */
       // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
       Cursor<IndexRow> range(IndexSearchRow lowerBound, IndexSearchRow upperBound);
   ```
   1. Actually, there is no keys in index. All rows are unique because they contains full keys. 
   I'm not sure any value are needed here (in terms of key-value store), just keys.
   Can we write just empty byte[] as value if 'null'-value is not supported.
   2. Put/remove will contains all the fields: indexed and key fields. Foreseeing a transactional support, the version might be part of key, and remove operation might be able to clean-up certain version of row. 
   3. Agree with separate class for range args (I'd call it "IndexSearchRow" for now), because it's structure may differs.
   4. I'd hide "SearchRow" under the get method: e.g. IndexRow.getDataRowKey(). Maybe we will store a whole key "as is" inside the IndexRow.
   5. Most likely we will want to get indexed columns values from an IndexRow. It is required for index-only scans, when we could return column value without retrieving a DataRow from storage.
   
   @tledkov-gridgain any objections from your side?




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r766743157



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.rocksdb.ColumnFamily;
+import org.apache.ignite.internal.rocksdb.RocksIteratorAdapter;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.IndexRowFactory;
+import org.apache.ignite.internal.storage.index.IndexRowPrefix;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexStorage;
+import org.apache.ignite.internal.util.Cursor;
+import org.jetbrains.annotations.Nullable;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.RocksIterator;
+
+/**
+ * {@link SortedIndexStorage} implementation based on RocksDB.
+ */
+public class RocksDbSortedIndexStorage implements SortedIndexStorage {
+    private final ColumnFamily indexCf;
+
+    private final SortedIndexDescriptor descriptor;
+
+    private final IndexRowFactory indexRowFactory;
+
+    private final IndexRowDeserializer indexRowDeserializer;
+
+    /**
+     * Creates a new Index storage.
+     *
+     * @param indexCf Column Family for storing the data.
+     * @param descriptor Index descriptor.
+     */
+    public RocksDbSortedIndexStorage(ColumnFamily indexCf, SortedIndexDescriptor descriptor) {
+        this.indexCf = indexCf;
+        this.descriptor = descriptor;
+        this.indexRowFactory = new BinaryIndexRowFactory(descriptor);
+        this.indexRowDeserializer = new BinaryIndexRowDeserializer(descriptor);
+    }
+
+    @Override
+    public SortedIndexDescriptor indexDescriptor() {
+        return descriptor;
+    }
+
+    @Override
+    public IndexRowFactory indexKeyFactory() {
+        return indexRowFactory;
+    }
+
+    @Override
+    public IndexRowDeserializer indexRowDeserializer() {
+        return indexRowDeserializer;
+    }
+
+    @Override
+    public void put(IndexRow row) {
+        try {
+            indexCf.put(row.rowBytes(), row.primaryKey().keyBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while adding data to Rocks DB", e);
+        }
+    }
+
+    @Override
+    public void remove(IndexRow key) {
+        try {
+            indexCf.delete(key.rowBytes());
+        } catch (RocksDBException e) {
+            throw new StorageException("Error while removing data from Rocks DB", e);
+        }
+    }
+
+    @Override
+    public Cursor<IndexRow> range(IndexRowPrefix lowerBound, IndexRowPrefix upperBound) {
+        RocksIterator iter = indexCf.newIterator();
+
+        iter.seekToFirst();

Review comment:
       Unfortunately, no. `BinaryRow` serialization places columns differently than specified in the schema. This means that some column, that is missing in the prefix, can actually be the first bytes of the serialized row. What I'm trying to say is that some prefix column values might not serialize as a byte prefix of the serialized row =( 




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768624660



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/BinaryRowComparator.java
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import static java.util.Comparator.comparing;
+import static java.util.Comparator.comparingDouble;
+import static java.util.Comparator.comparingInt;
+import static java.util.Comparator.comparingLong;
+import static org.apache.ignite.internal.storage.rocksdb.index.ComparatorUtils.comparingNull;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.Arrays;
+import java.util.Comparator;
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.NativeTypeSpec;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.rocksdb.AbstractComparator;
+import org.rocksdb.ComparatorOptions;
+
+/**
+ * Naive RocksDB comparator implementation that fully de-serializes the passed data and compares the index columns one after the other.
+ */
+public class BinaryRowComparator extends AbstractComparator {
+    /**
+     * Actual comparator implementation.
+     */
+    private final Comparator<ByteBuffer> innerComparator;
+
+    /**
+     * Options needed for resource management.
+     */
+    private final ComparatorOptions options;
+
+    /**
+     * Creates a RocksDB comparator for a Sorted Index identified by the given descriptor.
+     */
+    public BinaryRowComparator(SortedIndexDescriptor descriptor) {
+        this(descriptor, new ComparatorOptions());
+    }
+
+    /**
+     * Internal constructor for capturing the {@code options} parameter for resource management purposes.
+     */
+    private BinaryRowComparator(SortedIndexDescriptor descriptor, ComparatorOptions options) {
+        super(options);
+
+        innerComparator = comparing(
+                byteBuffer -> new ByteBufferRow(byteBuffer.order(ByteOrder.LITTLE_ENDIAN)),
+                binaryRowComparator(descriptor)
+        );
+
+        this.options = options;
+    }
+
+    /**
+     * Creates a comparator for comparing two {@link BinaryRow}s by converting them into {@link Row}s.
+     */
+    private static Comparator<BinaryRow> binaryRowComparator(SortedIndexDescriptor descriptor) {
+        return comparing(
+                binaryRow -> new Row(descriptor.asSchemaDescriptor(), binaryRow),
+                rowComparator(descriptor)
+        );
+    }
+
+    /**
+     * Creates a comparator that compares two {@link Row}s by comparing individual columns.
+     */
+    private static Comparator<Row> rowComparator(SortedIndexDescriptor descriptor) {
+        return descriptor.indexRowColumns().stream()
+                .map(columnDescriptor -> {
+                    Column column = columnDescriptor.column();
+
+                    Comparator<Row> columnComparator = columnComparator(column);
+
+                    if (columnDescriptor.nullable()) {
+                        columnComparator = comparingNull(
+                                row -> row.hasNullValue(column.schemaIndex(), column.type().spec()) ? null : row,
+                                columnComparator
+                        );
+                    }
+
+                    return columnDescriptor.asc() ? columnComparator : columnComparator.reversed();
+                })
+                .reduce(Comparator::thenComparing)
+                .orElseThrow();
+    }
+
+    /**
+     * Creates a comparator for comparing table columns.
+     */
+    private static Comparator<Row> columnComparator(Column column) {
+        int schemaIndex = column.schemaIndex();
+
+        NativeTypeSpec typeSpec = column.type().spec();
+
+        switch (typeSpec) {
+            case INT8:
+                return (row1, row2) -> {
+                    byte value1 = row1.byteValue(schemaIndex);
+                    byte value2 = row2.byteValue(schemaIndex);
+
+                    return Byte.compare(value1, value2);
+                };
+
+            case INT16:
+                return (row1, row2) -> {
+                    short value1 = row1.shortValue(schemaIndex);
+                    short value2 = row2.shortValue(schemaIndex);
+
+                    return Short.compare(value1, value2);
+                };
+
+            case INT32:
+                return comparingInt(row -> row.intValue(schemaIndex));
+
+            case INT64:
+                return comparingLong(row -> row.longValue(schemaIndex));
+
+            case FLOAT:
+                return (row1, row2) -> {
+                    float value1 = row1.floatValue(schemaIndex);
+                    float value2 = row2.floatValue(schemaIndex);
+
+                    return Float.compare(value1, value2);
+                };
+
+            case DOUBLE:
+                return comparingDouble(row -> row.doubleValue(schemaIndex));
+
+            case BYTES:
+                return comparing(row -> row.bytesValue(schemaIndex), Arrays::compare);
+
+            case BITMASK:
+                return comparing(row -> row.bitmaskValue(schemaIndex).toLongArray(), Arrays::compare);
+
+            // all other types implement Comparable
+            case DECIMAL:
+            case UUID:
+            case STRING:
+            case NUMBER:
+            case TIMESTAMP:
+            case DATE:
+            case TIME:
+            case DATETIME:
+                return comparing(row -> (Comparable) typeSpec.objectValue(row, schemaIndex));
+
+            default:
+                throw new IllegalArgumentException(String.format(
+                        "Unsupported column schema for creating a sorted index. Column name: %s, column type: %s",
+                        column.name(), column.type()
+                ));
+        }
+    }
+
+    @Override

Review comment:
       Is that required by the code style?




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768624472



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/BinaryIndexRowDeserializer.java
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.storage.index.IndexRow;
+import org.apache.ignite.internal.storage.index.IndexRowDeserializer;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor.ColumnDescriptor;
+
+/**
+ * {@link IndexRowDeserializer} implementation that uses {@link BinaryRow} infrastructure for deserialization purposes.
+ */
+class BinaryIndexRowDeserializer implements IndexRowDeserializer {
+    private final SortedIndexDescriptor descriptor;
+
+    BinaryIndexRowDeserializer(SortedIndexDescriptor descriptor) {
+        this.descriptor = descriptor;
+    }
+
+    @Override
+    public Object[] indexedColumnValues(IndexRow indexRow) {
+        var row = new Row(descriptor.asSchemaDescriptor(), new ByteBufferRow(indexRow.rowBytes()));
+
+        return descriptor.indexRowColumns().stream()

Review comment:
       I have no idea




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768760508



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.tostring.S;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        private final boolean nullable;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn, boolean nullable) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+            this.nullable = nullable;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        /**
+         * Returns {@code true} if this column can contain null values or {@code false} otherwise.
+         */
+        public boolean nullable() {
+            return nullable;
+        }
+
+        @Override
+        public String toString() {
+            return S.toString(this);
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream();
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition
+                .map(columnName -> {
+                    ColumnView columnView = tableConfig.columns().get(columnName);
+
+                    assert columnView != null : "Incorrect index column configuration. " + columnName + " column does not exist";
+
+                    return columnView;
+                })
+                .collect(toList());
+
+        schemaDescriptor = createSchemaDescriptor(indexKeyColumnViews);
+
+        columns = Arrays.stream(schemaDescriptor.keyColumns().columns())
+                .sorted(Comparator.comparingInt(Column::columnOrder))
+                .map(column -> {
+                    IndexColumnView columnView = indexColumns.get(column.name());
+
+                    // if the index config does not contain this column - it's a column from the Primary Key
+                    boolean indexedColumn = columnView != null;
+
+                    // PK columns are always sorted in ascending order
+                    boolean asc = !indexedColumn || columnView.asc();
+
+                    return new ColumnDescriptor(column, asc, indexedColumn, column.nullable());
+                })
+                .collect(toUnmodifiableList());
+    }
+
+    /**
+     * Creates a {@link SchemaDescriptor} from a list of index key columns.
+     */
+    private static SchemaDescriptor createSchemaDescriptor(List<ColumnView> indexKeyColumnViews) {
+        Column[] keyColumns = new Column[indexKeyColumnViews.size()];
+
+        for (int i = 0; i < indexKeyColumnViews.size(); ++i) {
+            ColumnView columnView = indexKeyColumnViews.get(i);
+
+            ColumnDefinition columnDefinition = SchemaConfigurationConverter.convert(columnView);
+
+            keyColumns[i] = SchemaDescriptorConverter.convert(i, columnDefinition);
+        }
+
+        return new SchemaDescriptor(0, keyColumns, new Column[0]);
+    }
+
+    /**
+     * Returns this index' name.
+     */
+    public String name() {
+        return name;
+    }
+
+    /**
+     * Returns the Column Descriptors that comprise a row of this index (indexed columns + primary key columns).
+     */
+    public List<ColumnDescriptor> indexRowColumns() {
+        return columns;
+    }
+
+    /**
+     * Converts this Descriptor into an equivalent {@link SchemaDescriptor}.
+     *
+     * <p>The resulting {@code SchemaDescriptor} will have empty {@link SchemaDescriptor#valueColumns()} and its

Review comment:
       @SammyVimes do you mean `<p>` tag?
   Seems, no, closing tag is not required.
   Otherwise, let's fix checkstyle rules.




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r763172076



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;

Review comment:
       Isn't this a bit premature? This field isn't even used in index-based accesses.




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#issuecomment-986611932


   As long as we lookup keys in IndexStorage, maybe we can store whole keychunk "as is" and never reassemble it?
   So, indexRow schema can have the same key-columns, and only indexed value columns if used.
   Thus, row assembler can be reused and key-bytes can be easily extracted from index row.
   
   Why does IndexKey hold byte[] as it is never intended to be stored?, regarding an external comparator will be used in anyway?


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r764947866



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        ColumnDescriptor(Column column, boolean asc) {
+            this.column = column;
+            this.asc = asc;
+        }
+
+        public Column column() {
+            return column;
+        }
+
+        public boolean asc() {
+            return asc;
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        SortedIndexView indexConfig = (SortedIndexView) tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = indexConfig.columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream()
+                .map(indexColumns::get)
+                .map(IndexColumnView::name);
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition

Review comment:
       sorry, what do you mean? Why do you think this stream is not ordered?




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] AMashenkov commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r764755028



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexStorage.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Storage for a Sorted Index.
+ *
+ * <p>This storage serves as a sorted mapping from a subset of a table's columns (a.k.a. index columns) to a {@link SearchRow}
+ * from a {@link org.apache.ignite.internal.storage.PartitionStorage} from the same table.
+ *
+ * @see org.apache.ignite.schema.definition.index.SortedIndexDefinition
+ */
+public interface SortedIndexStorage extends AutoCloseable {
+    /**
+     * Returns a factory for creating index keys for this storage.
+     */
+    IndexKeyFactory indexKeyFactory();
+
+    /**
+     * Returns the Index Descriptor of this storage.
+     */
+    SortedIndexDescriptor indexDescriptor();
+
+    /**
+     * Adds the given index key and {@link SearchRow} to the index.
+     *
+     * <p>Putting a new value under the same key will overwrite the previous associated value.
+     */
+    void put(IndexKey key, SearchRow value);
+
+    /**
+     * Removes the given key from the index.
+     *
+     * <p>Removing a non-existent key is a no-op.
+     */
+    void remove(IndexKey key);
+
+    /**
+     * Returns a range of index values between the lower bound (inclusive) and the upper bound (inclusive).
+     */
+    // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
+    Cursor<SearchRow> range(IndexKeyPrefix lowerBound, IndexKeyPrefix upperBound);

Review comment:
       Can we have smth like this.
   ```suggestion
       /**
        * Adds the given index row to the index.
        */
       boolean put(IndexRow row);
   
       /**
        * Removes the given row from the index.
        */
       boolean remove(IndexRow row);
   
       /**
        * Returns a range of index rows between the lower bound (inclusive) and the upper bound (inclusive).
        */
       // TODO: add options https://issues.apache.org/jira/browse/IGNITE-16059
       Cursor<IndexRow> range(IndexSearchRow lowerBound, IndexSearchRow upperBound);
   ```
   1. Actually, there is no keys in index. All rows are unique because they contains full keys. 
   I'm not sure any value are needed here (in terms of key-value store), just keys.
   Can we write just empty byte[] as value if 'null'-value is not supported.
   2. Put/remove will contains all the fields: indexed and key fields. Foreseeing a transactional support, the version might be part of key, and remove operation might be able to clean-up certain version of row. 
   3. Agree with separate class for range args (I'd call it "IndexSearchRow" for now), because it's structure may differs.
   4. I'd hide "SearchRow" under the get method: e.g. IndexRow.getDataRowKey(). Maybe we will store a whole key "as is" inside the IndexRow.
   5. Most likely we will want to get indexed columns values from an IndexRow. It is required for index-only scans, when we could return column value without retrieving a DataRow from storage.
   6. Returning 'boolean' may be useful. Does't it costs anything or easy to do?
   
   @tledkov-gridgain any objections from your side?




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768624210



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/TableStorage.java
##########
@@ -52,10 +52,32 @@
      *
      * @param partId Partition id.
      * @throws IllegalArgumentException If partition id is out of bounds.
-     * @throws StorageException If an error has occurred during the partition destruction.
+     * @throws StorageException         If an error has occurred during the partition destruction.
      */
     void dropPartition(int partId) throws StorageException;
 
+    /**
+     * Creates or returns an already created Sorted Index with the given name.
+     *
+     * <p>A prerequisite for calling this method is to have the index already configured under the same name in the Table Configuration

Review comment:
       why?




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] SammyVimes commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
SammyVimes commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r768640524



##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/BinaryRowComparator.java
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.rocksdb.index;
+
+import static java.util.Comparator.comparing;
+import static java.util.Comparator.comparingDouble;
+import static java.util.Comparator.comparingInt;
+import static java.util.Comparator.comparingLong;
+import static org.apache.ignite.internal.storage.rocksdb.index.ComparatorUtils.comparingNull;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.Arrays;
+import java.util.Comparator;
+import org.apache.ignite.internal.schema.BinaryRow;
+import org.apache.ignite.internal.schema.ByteBufferRow;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.NativeTypeSpec;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.storage.index.SortedIndexDescriptor;
+import org.rocksdb.AbstractComparator;
+import org.rocksdb.ComparatorOptions;
+
+/**
+ * Naive RocksDB comparator implementation that fully de-serializes the passed data and compares the index columns one after the other.
+ */
+public class BinaryRowComparator extends AbstractComparator {
+    /**
+     * Actual comparator implementation.
+     */
+    private final Comparator<ByteBuffer> innerComparator;
+
+    /**
+     * Options needed for resource management.
+     */
+    private final ComparatorOptions options;
+
+    /**
+     * Creates a RocksDB comparator for a Sorted Index identified by the given descriptor.
+     */
+    public BinaryRowComparator(SortedIndexDescriptor descriptor) {
+        this(descriptor, new ComparatorOptions());
+    }
+
+    /**
+     * Internal constructor for capturing the {@code options} parameter for resource management purposes.
+     */
+    private BinaryRowComparator(SortedIndexDescriptor descriptor, ComparatorOptions options) {
+        super(options);
+
+        innerComparator = comparing(
+                byteBuffer -> new ByteBufferRow(byteBuffer.order(ByteOrder.LITTLE_ENDIAN)),
+                binaryRowComparator(descriptor)
+        );
+
+        this.options = options;
+    }
+
+    /**
+     * Creates a comparator for comparing two {@link BinaryRow}s by converting them into {@link Row}s.
+     */
+    private static Comparator<BinaryRow> binaryRowComparator(SortedIndexDescriptor descriptor) {
+        return comparing(
+                binaryRow -> new Row(descriptor.asSchemaDescriptor(), binaryRow),
+                rowComparator(descriptor)
+        );
+    }
+
+    /**
+     * Creates a comparator that compares two {@link Row}s by comparing individual columns.
+     */
+    private static Comparator<Row> rowComparator(SortedIndexDescriptor descriptor) {
+        return descriptor.indexRowColumns().stream()
+                .map(columnDescriptor -> {
+                    Column column = columnDescriptor.column();
+
+                    Comparator<Row> columnComparator = columnComparator(column);
+
+                    if (columnDescriptor.nullable()) {
+                        columnComparator = comparingNull(
+                                row -> row.hasNullValue(column.schemaIndex(), column.type().spec()) ? null : row,
+                                columnComparator
+                        );
+                    }
+
+                    return columnDescriptor.asc() ? columnComparator : columnComparator.reversed();
+                })
+                .reduce(Comparator::thenComparing)
+                .orElseThrow();
+    }
+
+    /**
+     * Creates a comparator for comparing table columns.
+     */
+    private static Comparator<Row> columnComparator(Column column) {
+        int schemaIndex = column.schemaIndex();
+
+        NativeTypeSpec typeSpec = column.type().spec();
+
+        switch (typeSpec) {
+            case INT8:
+                return (row1, row2) -> {
+                    byte value1 = row1.byteValue(schemaIndex);
+                    byte value2 = row2.byteValue(schemaIndex);
+
+                    return Byte.compare(value1, value2);
+                };
+
+            case INT16:
+                return (row1, row2) -> {
+                    short value1 = row1.shortValue(schemaIndex);
+                    short value2 = row2.shortValue(schemaIndex);
+
+                    return Short.compare(value1, value2);
+                };
+
+            case INT32:
+                return comparingInt(row -> row.intValue(schemaIndex));
+
+            case INT64:
+                return comparingLong(row -> row.longValue(schemaIndex));
+
+            case FLOAT:
+                return (row1, row2) -> {
+                    float value1 = row1.floatValue(schemaIndex);
+                    float value2 = row2.floatValue(schemaIndex);
+
+                    return Float.compare(value1, value2);
+                };
+
+            case DOUBLE:
+                return comparingDouble(row -> row.doubleValue(schemaIndex));
+
+            case BYTES:
+                return comparing(row -> row.bytesValue(schemaIndex), Arrays::compare);
+
+            case BITMASK:
+                return comparing(row -> row.bitmaskValue(schemaIndex).toLongArray(), Arrays::compare);
+
+            // all other types implement Comparable
+            case DECIMAL:
+            case UUID:
+            case STRING:
+            case NUMBER:
+            case TIMESTAMP:
+            case DATE:
+            case TIME:
+            case DATETIME:
+                return comparing(row -> (Comparable) typeSpec.objectValue(row, schemaIndex));
+
+            default:
+                throw new IllegalArgumentException(String.format(
+                        "Unsupported column schema for creating a sorted index. Column name: %s, column type: %s",
+                        column.name(), column.type()
+                ));
+        }
+    }
+
+    @Override

Review comment:
       Well, AFAIK every method should have a javadoc




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-3] sashapolo commented on a change in pull request #468: IGNITE-15885 Implement RocksDB-based Sorted Index Storage

Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #468:
URL: https://github.com/apache/ignite-3/pull/468#discussion_r770415057



##########
File path: modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/SortedIndexDescriptor.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.storage.index;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toUnmodifiableList;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.IndexColumnView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexView;
+import org.apache.ignite.configuration.schemas.table.TableIndexView;
+import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaDescriptor;
+import org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter;
+import org.apache.ignite.internal.schema.configuration.SchemaDescriptorConverter;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.tostring.S;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+
+/**
+ * Descriptor for creating a Sorted Index Storage.
+ *
+ * @see SortedIndexStorage
+ */
+public class SortedIndexDescriptor {
+    /**
+     * Descriptor of a Sorted Index column (column name and column sort order).
+     */
+    public static class ColumnDescriptor {
+        private final Column column;
+
+        private final boolean asc;
+
+        private final boolean indexedColumn;
+
+        private final boolean nullable;
+
+        ColumnDescriptor(Column column, boolean asc, boolean indexedColumn, boolean nullable) {
+            this.column = column;
+            this.asc = asc;
+            this.indexedColumn = indexedColumn;
+            this.nullable = nullable;
+        }
+
+        /**
+         * Returns a column descriptor.
+         */
+        public Column column() {
+            return column;
+        }
+
+        /**
+         * Returns {@code true} if this column is sorted in ascending order or {@code false} otherwise.
+         */
+        public boolean asc() {
+            return asc;
+        }
+
+        /**
+         * Returns {@code true} if this column was explicitly marked as an indexed column or {@code false} if it is a part of a Primary Key
+         * appended for uniqueness.
+         */
+        public boolean indexedColumn() {
+            return indexedColumn;
+        }
+
+        /**
+         * Returns {@code true} if this column can contain null values or {@code false} otherwise.
+         */
+        public boolean nullable() {
+            return nullable;
+        }
+
+        @Override
+        public String toString() {
+            return S.toString(this);
+        }
+    }
+
+    private final String name;
+
+    private final List<ColumnDescriptor> columns;
+
+    private final SchemaDescriptor schemaDescriptor;
+
+    /**
+     * Creates an Index Descriptor from a given Table Configuration.
+     *
+     * @param name        index name.
+     * @param tableConfig table configuration.
+     */
+    public SortedIndexDescriptor(String name, TableView tableConfig) {
+        this.name = name;
+
+        TableIndexView indexConfig = tableConfig.indices().get(name);
+
+        if (indexConfig == null) {
+            throw new StorageException(String.format("Index configuration for \"%s\" could not be found", name));
+        }
+
+        if (!(indexConfig instanceof SortedIndexView)) {
+            throw new StorageException(String.format(
+                    "Index \"%s\" is not configured as a Sorted Index. Actual type: %s",
+                    name, indexConfig.type()
+            ));
+        }
+
+        // extract indexed column configurations from the table configuration
+        NamedListView<? extends IndexColumnView> indexColumns = ((SortedIndexView) indexConfig).columns();
+
+        Stream<String> indexColumnNames = indexColumns.namedListKeys().stream();
+
+        // append the primary key to guarantee index key uniqueness
+        Stream<String> primaryKeyColumnNames = Arrays.stream(tableConfig.primaryKey().columns());
+
+        List<ColumnView> indexKeyColumnViews = Stream.concat(indexColumnNames, primaryKeyColumnNames)
+                .distinct() // remove Primary Key columns if they are already present in the index definition
+                .map(columnName -> {
+                    ColumnView columnView = tableConfig.columns().get(columnName);
+
+                    assert columnView != null : "Incorrect index column configuration. " + columnName + " column does not exist";
+
+                    return columnView;
+                })
+                .collect(toList());
+
+        schemaDescriptor = createSchemaDescriptor(indexKeyColumnViews);
+
+        columns = Arrays.stream(schemaDescriptor.keyColumns().columns())
+                .sorted(Comparator.comparingInt(Column::columnOrder))

Review comment:
       yes, but I later need the correct `schemaIndex` inside the `BinaryRowComparator` for example. So, the only problem is that `indexKeyColumnViews` do not contain fully initialized `Column` objects, that are later needed. 




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org