Skip to content

Conversation

XuPeng-SH
Copy link
Contributor

@XuPeng-SH XuPeng-SH commented Oct 14, 2025

User description

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #22590

What this PR does / why we need it:

add some util api for sdk


PR Type

Enhancement, Documentation


Description

  • Add secondary index verification utilities for consistency checks

  • Implement index table retrieval methods (get all indexes, get by name)

  • Add comprehensive documentation and examples for index verification

  • Include online tests for sync and async index verification


Diagram Walkthrough

flowchart LR
  A["index_utils.py"] --> B["Client"]
  A --> C["AsyncClient"]
  B --> D["Index Verification Methods"]
  C --> E["Async Index Methods"]
  D --> F["Tests & Docs"]
  E --> F
Loading

File Walkthrough

Relevant files
Enhancement
4 files
index_utils.py
New module with index verification SQL builders and processors
+121/-0 
client.py
Add three index verification methods to Client class         
+96/-2   
async_client.py
Add async versions of index verification methods                 
+94/-3   
Makefile
Add lint-fix target for auto-fixing code issues                   
+14/-0   
Tests
2 files
test_index_verification_online.py
Online tests for synchronous index verification functionality
+194/-0 
test_async_index_verification_online.py
Online tests for async index verification functionality   
+259/-0 
Documentation
4 files
index_verification_guide.rst
Complete guide for secondary index verification features 
+272/-0 
examples.rst
Add index verification examples to documentation                 
+84/-0   
index.rst
Add index verification guide to documentation TOC               
+1/-0     
README.md
Add index verification feature description and examples   
+25/-0   
Configuration changes
1 files
pyproject.toml
Bump version from 0.1.2 to 0.1.3                                                 
+1/-1     
Formatting
6 files
exceptions.py
Remove redundant pass statements from exception classes   
+0/-24   
metadata.py
Reorder imports for consistency                                                   
+3/-2     
async_metadata_manager.py
Reorder imports for consistency                                                   
+3/-1     
connection_hooks.py
Reorder imports for consistency                                                   
+1/-0     
moctl.py
Remove redundant pass statement from exception class         
+0/-2     
version.py
Remove redundant pass statement from exception class         
+0/-2     
Bug fix
1 files
__init__.py
Add FulltextParserType to exports list                                     
+1/-0     

@mergify mergify bot added kind/enhancement kind/documentation Improvements or additions to documentation labels Oct 14, 2025
Copy link

qodo-merge-pro bot commented Oct 14, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link

qodo-merge-pro bot commented Oct 14, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Remove duplicated test class definition

Remove the duplicated TestAsyncIndexVerificationOnline class definition in
test_async_index_verification_online.py and merge its contents into a single
class to ensure all tests are discovered and executed correctly.

clients/python/tests/online/test_async_index_verification_online.py [33-107]

 class TestAsyncIndexVerificationOnline:
     """Online tests for async secondary index verification"""
 
     @pytest_asyncio.fixture(scope="function")
     async def async_client_with_test_table(self):
         """Create AsyncClient with test table"""
         host, port, user, password, database = online_config.get_connection_params()
         client = AsyncClient()
         await client.connect(host=host, port=port, user=user, password=password, database=database)
 
         test_table = "test_async_index_table"
 
         try:
             # Define ORM model with secondary indexes
             Base = declarative_base()
 
             class TestTable(Base):
                 __tablename__ = test_table
 
                 id = Column(Integer, primary_key=True)
                 name = Column(String(100))
                 category = Column(String(50))
                 value = Column(Integer)
 
                 # Define secondary indexes
                 __table_args__ = (
                     Index('idx_async_name', 'name'),
                     Index('idx_async_category', 'category'),
                     Index('idx_async_value', 'value'),
                 )
 
             # Drop table if exists and create new one using SDK API
             try:
                 await client.drop_table(TestTable)
             except:
                 pass
 
             # Create table with indexes using SDK API
             await client.create_table(TestTable)
 
             # Insert test data using SDK API
             test_data = [
                 {'id': i, 'name': f'async_name_{i}', 'category': f'cat_{i % 5}', 'value': i * 10} for i in range(1, 101)
             ]
             await client.batch_insert(TestTable, test_data)
 
             yield client, test_table, TestTable
 
         finally:
             # Clean up using SDK API
             try:
                 await client.drop_table(TestTable)
                 await client.disconnect()
             except Exception as e:
                 print(f"Cleanup failed: {e}")
 
     @pytest.mark.asyncio
     async def test_async_get_secondary_index_tables(self, async_client_with_test_table):
         """Test async getting all secondary index table names"""
         client, test_table, TestTable = async_client_with_test_table
 
         index_tables = await client.get_secondary_index_tables(test_table)
 
         # Should have 3 secondary indexes
         assert len(index_tables) == 3, f"Expected 3 indexes, got {len(index_tables)}"
 
         # All index tables should start with __mo_index_secondary_
         for index_table in index_tables:
             assert index_table.startswith(
                 '__mo_index_secondary_'
             ), f"Index table name should start with '__mo_index_secondary_', got {index_table}"
 
+    @pytest.mark.asyncio
+    async def test_async_get_secondary_index_table_by_name(self, async_client_with_test_table):
+        """Test async getting physical table name of a secondary index by its name"""
+        client, test_table, TestTable = async_client_with_test_table
 
-class TestAsyncIndexVerificationOnline:
-    """Online tests for async secondary index verification"""
-...
+        # Test getting idx_async_name
+        idx_name_table = await client.get_secondary_index_table_by_name(test_table, 'idx_async_name')
+        assert idx_name_table is not None, "idx_async_name should exist"
+        assert idx_name_table.startswith('__mo_index_secondary_')
 
+        # Test getting idx_async_category
+        idx_category_table = await client.get_secondary_index_table_by_name(test_table, 'idx_async_category')
+        assert idx_category_table is not None, "idx_async_category should exist"
+        assert idx_category_table.startswith('__mo_index_secondary_')
+
+        # Test getting idx_async_value
+        idx_value_table = await client.get_secondary_index_table_by_name(test_table, 'idx_async_value')
+        assert idx_value_table is not None, "idx_async_value should exist"
+        assert idx_value_table.startswith('__mo_index_secondary_')
+
+        # Test non-existent index
+        non_existent = await client.get_secondary_index_table_by_name(test_table, 'idx_non_existent')
+        assert non_existent is None, "Non-existent index should return None"
+
+    @pytest.mark.asyncio
+    async def test_async_verify_table_index_counts(self, async_client_with_test_table):
+        """Test async verify_table_index_counts when counts match"""
+        client, test_table, TestTable = async_client_with_test_table
+
+        # All indexes should have the same count as the main table
+        count = await client.verify_table_index_counts(test_table)
+
+        # Should return 100 (number of inserted rows)
+        assert count == 100, f"Expected count 100, got {count}"
+
+        # Verify count matches query result using SDK API
+        actual_count = await client.query(TestTable).count()
+        assert count == actual_count, "Verify result should match actual count"
+
+    @pytest.mark.asyncio
+    async def test_async_verify_table_without_indexes(self, async_client_with_test_table):
+        """Test async verify_table_index_counts on table without secondary indexes"""
+        client, test_table, TestTable = async_client_with_test_table
+
+        # Define a simple model without secondary indexes
+        Base = declarative_base()
+
+        class SimpleTable(Base):
+            __tablename__ = "test_async_simple_table"
+            id = Column(Integer, primary_key=True)
+            value = Column(Integer)
+
+        # Create table using SDK API
+        try:
+            await client.drop_table(SimpleTable)
+        except:
+            pass
+
+        await client.create_table(SimpleTable)
+
+        # Insert data using SDK API
+        test_data = [{'id': 1, 'value': 100}, {'id': 2, 'value': 200}]
+        await client.batch_insert(SimpleTable, test_data)
+
+        # Verification should succeed and return count
+        count = await client.verify_table_index_counts("test_async_simple_table")
+        assert count == 2, f"Expected count 2, got {count}"
+
+        # Cleanup using SDK API
+        await client.drop_table(SimpleTable)
+
+    @pytest.mark.asyncio
+    async def test_async_index_table_mapping(self, async_client_with_test_table):
+        """Test that all indexes can be retrieved both ways (async)"""
+        client, test_table, TestTable = async_client_with_test_table
+
+        # Get all index tables
+        all_index_tables = await client.get_secondary_index_tables(test_table)
+
+        # Get each index by name
+        index_names = ['idx_async_name', 'idx_async_category', 'idx_async_value']
+        retrieved_tables = []
+
+        for index_name in index_names:
+            table = await client.get_secondary_index_table_by_name(test_table, index_name)
+            assert table is not None, f"Index {index_name} should exist"
+            retrieved_tables.append(table)
+
+        # All retrieved tables should be in the all_index_tables list
+        for table in retrieved_tables:
+            assert table in all_index_tables, f"Retrieved table {table} should be in all index tables"
+
+        # Both lists should have the same length
+        assert len(retrieved_tables) == len(all_index_tables), "Number of retrieved tables should match total index tables"
+

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a duplicated test class TestAsyncIndexVerificationOnline, which is a significant bug in the test suite that would cause some tests to be skipped. Fixing this is critical for ensuring test coverage for the new feature.

Medium
  • Update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/documentation Improvements or additions to documentation kind/enhancement Review effort 3/5 size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants