Add the first connector test and fix all issues #3 by mohsaka · Pull Request #5 · mohsaka/presto

mohsaka · 2025-03-11T17:15:35Z

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* ... 
* ... 

Hive Connector Changes
* ... 
* ...

If release note is NOT required, use:

== NO RELEASE NOTE ==

Changes adapted from trino/PR#12910 Original commit: f0508a7ab420449c6e2960ecf1d0a8d7058242da Author: kasiafi Modifications were made to adapt to Presto including: Addition of test case under TestJdbcDistributedQueries.java. Removal of Trino JDBC test cases.

Changes adapted from trino/PR#12951 Original commit: b602e4e0065d00a2c9b1f645cf7ed2905bdd6078 Original commit: 98fc1ee8b29fca86f2a1b3abe4989524940333a6 Original commit: 0cec709ae1b6c707ca4810cdef8a852253167ab1 Author: kasiafi Modifications were made to adapt to Presto including: Addition of other Trino Mock testing classes. Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>

mohsaka · 2025-03-11T17:22:41Z

Issue:
NoOpTransactionManager was being passed down from Analyzer.

Fix:
Get the TransactionManager from metadata.getFunctionAndTypeManager().getTransactionManager().

mohsaka · 2025-03-11T17:24:38Z

Issue:
TransactionManager was not registering the catalogs by connector id.

Fix:
Per the comment

            // a call to getRequiredCatalogHandle() is necessary so that the catalog is recorded by the TransactionManager
            ConnectorTransactionHandle transactionHandle = transactionManager.getConnectorTransaction(
                    session.getRequiredTransactionId(),
                    connectorId);

We don't have a getRequiredCatalogHandle. So we are calling, getOptionalCatalogMetadata. We then use this metadata to get the connector ID.

            // a call to getOptionalCatalogMetadata() is necessary so that the catalog is recorded by the TransactionManager
            CatalogMetadata registrationCatalogMetadata = transactionManager.getOptionalCatalogMetadata(session.getRequiredTransactionId(), connectorId.getCatalogName()).orElseThrow(() -> new IllegalStateException("Missing catalog metadata"));
            ConnectorTransactionHandle transactionHandle = transactionManager.getConnectorTransaction(
                    session.getRequiredTransactionId(), registrationCatalogMetadata.getConnectorId());

mohsaka · 2025-03-11T17:29:41Z

Issue:

ValidateDependenciesChecker is returning
throw new UnsupportedOperationException("not yet implemented: " + node.getClass().getName());

In Trino, we use a TypeValidator instead of the ValidateDependenciesChecker. This TypeValidator's Visitor is a SimplePlanVisitor while the Presto ValidateDependenciesChecker Visitor is a PlanChecker.Checker. It also overwrites the visitPlan with one that is not yet implemented.

Brought the visitPlan from SimplePlanVisitor into ValidateDependenciesChecker Visitor. There are currently no sources for the PlanNode at this step.

NOTE:
Not sure if this is the proper fix.

mohsaka · 2025-03-11T17:31:03Z

Issue:

Caused by: java.lang.UnsupportedOperationException: Unsupported plan node TableFunctionNode
	at com.facebook.presto.sql.planner.optimizations.UnaliasSymbolReferences$Rewriter.visitPlan(UnaliasSymbolReferences.java:702)
	at com.facebook.presto.sql.planner.optimizations.UnaliasSymbolReferences$Rewriter.visitPlan(UnaliasSymbolReferences.java:140)
	at com.facebook.presto.sql.planner.plan.InternalPlanVisitor.visitTableFunction(InternalPlanVisitor.java:147)
	at com.facebook.presto.sql.planner.plan.TableFunctionNode.accept(TableFunctionNode.java:131)
	at com.facebook.presto.sql.planner.plan.InternalPlanNode.accept(InternalPlanNode.java:34)
	at com.facebook.presto.sql.planner.plan.SimplePlanRewriter$RewriteContext.rewrite(SimplePlanRewriter.java:92)
	at com.facebook.presto.sql.planner.plan.SimplePlanRewriter$RewriteContext.rewrite(SimplePlanRewriter.java:106)
	at com.facebook.presto.sql.planner.optimizations.UnaliasSymbolReferences$Rewriter.visitProject(UnaliasSymbolReferences.java:509)
	at com.facebook.presto.sql.planner.optimizations.UnaliasSymbolReferences$Rewriter.visitProject(UnaliasSymbolReferences.java:140)
	at com.facebook.presto.spi.plan.ProjectNode.accept(ProjectNode.java:108)

Fixed this and previous one via overriding visitTableFunction

mohsaka · 2025-03-11T21:31:33Z

Modified MockConnector with default layouts instead of not implemented. This is because Presto pushes down predicates on visitTableScan as well as visitFilter, while Trino only does it on visitFilter.
Modified getColumnMetadata in MockConnectorFactory to create column metadata on either tpch or MockConnectorColumn handles as Presto uses TPCHcolumnhandles in some tests with mockConnectors.

mohsaka · 2025-03-12T03:53:06Z

Current issue,

metadata.getTableMetadata is returning null.

Caused by: java.lang.NullPointerException
	at com.facebook.presto.metadata.MetadataManager.getTableMetadata(MetadataManager.java:482)

Fix.
Implement defaultGetColumns and call it in getTableMetadata instead of returning null.

mohsaka · 2025-03-12T17:55:38Z

Current issue,

Caused by: java.lang.ClassCastException: com.facebook.presto.testing.TestingHandle cannot be cast to com.facebook.presto.tpch.TpchTableLayoutHandle
	at com.facebook.presto.tpch.TpchSplitManager.getSplits(TpchSplitManager.java:51)
	at com.facebook.presto.split.SplitManager.getSplits(SplitManager.java:89)

This is caused by MockConnectorFactory using the TpchSplitManager.

        @Override
        public ConnectorSplitManager getSplitManager()
        {
            return new TpchSplitManager(context.getNodeManager(), 1);
        }

In Trino they use a different split manager

   @Override
   public ConnectorSplitManager getSplitManager()
   {
       return new ConnectorSplitManager()
       {
           @Override
           public ConnectorSplitSource getSplits(
                   ConnectorTransactionHandle transaction,
                   ConnectorSession session,
                   ConnectorTableHandle table,
                   DynamicFilter dynamicFilter,
                   Constraint constraint)
           {
               return new FixedSplitSource(MOCK_CONNECTOR_SPLIT);
           }

           @Override
           public ConnectorSplitSource getSplits(ConnectorTransactionHandle transaction, ConnectorSession session, ConnectorTableFunctionHandle functionHandle)
           {
               ConnectorSplitSource splits = tableFunctionSplitsSources.apply(functionHandle);
               return requireNonNull(splits, "missing ConnectorSplitSource for table function handle " + functionHandle.getClass().getSimpleName());
           }
       };
   }
   ```

mohsaka · 2025-03-12T19:35:28Z

Caused by: java.lang.NullPointerException
	at com.facebook.presto.execution.scheduler.nodeSelection.SimpleNodeSelector.computeAssignments(SimpleNodeSelector.java:156)
	at com.facebook.presto.execution.scheduler.DynamicSplitPlacementPolicy.computeAssignments(DynamicSplitPlacementPolicy.java:42)
	at com.facebook.presto.execution.scheduler.SourcePartitionedScheduler.schedule(SourcePartitionedScheduler.java:273)
	at com.facebook.presto.execution.scheduler.SourcePartitionedScheduler$1.schedule(SourcePartitionedScheduler.java:147)
	at com.facebook.presto.execution.scheduler.SqlQueryScheduler.schedule(SqlQueryScheduler.java:453)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:266)
	at java.util.concurrent.FutureTask.run(FutureTask.java)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:750)

This is caused by the previous change.

            @Override
            public NodeSelectionStrategy getNodeSelectionStrategy()
            {
                return null;
            }

Updated to

        public NodeSelectionStrategy getNodeSelectionStrategy()
       {
           return NO_PREFERENCE;
       }

From TestDriver's MockSplit

mohsaka · 2025-03-12T20:38:14Z

Current issue.

Caused by: java.lang.IllegalArgumentException: No connector for handle: system.simple_table of type class com.facebook.presto.connector.MockConnectorTableHandle
	at com.facebook.presto.metadata.HandleResolver.getId(HandleResolver.java:228)
	at com.facebook.presto.metadata.HandleResolver.getId(HandleResolver.java:85)
	at com.facebook.presto.metadata.AbstractTypedJacksonModule$InternalTypeResolver.idFromValueAndType(AbstractTypedJacksonModule.java:158)
	at com.facebook.presto.metadata.AbstractTypedJacksonModule$InternalTypeResolver.idFromValue(AbstractTypedJacksonModule.java:150)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeSerializerBase.idFromValue(TypeSerializerBase.java:92)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeSerializerBase._generateTypeId(TypeSerializerBase.java:77)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeSerializerBase.writeTypePrefix(TypeSerializerBase.java:45)
	at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeWithType(BeanSerializerBase.java:650)
	at com.facebook.presto.metadata.AbstractTypedJacksonModule$InternalTypeSerializer.serialize(AbstractTypedJacksonModule.java:115)
	at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:732)
	at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:772)
	... 20 more

Issue is caused by the HandleResolver not finding a MockConnectorColumnHandle resolver. This is because we were using a TpchColumnHandle resolver. Created MockHandleResolver class.

TODO: Future, we may need to separate out our Mock classes into our own as it seems like all of the usages of TPCH need to be removed from the Mock. I'm guessing that the Mock classes in Presto have diverged for a different use case than Trino's. So we will probably need our own.

mohsaka · 2025-03-13T01:48:19Z

Current issue

Caused by: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: No serializer found for class com.facebook.presto.connector.MockConnectorFactory$MockConnector$1 and no properties discovered to create BeanSerializer (to avoid exception, disable SerializationFeature.FAIL_ON_EMPTY_BEANS) (through reference chain: com.facebook.presto.sql.planner.PlanFragment["root"]->com.facebook.presto.spi.plan.TableScanNode["table"]->com.facebook.presto.spi.TableHandle["transaction"])
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:77)
	at com.fasterxml.jackson.databind.SerializerProvider.reportBadDefinition(SerializerProvider.java:1308)
	at com.fasterxml.jackson.databind.DatabindContext.reportBadDefinition(DatabindContext.java:414)
	at com.fasterxml.jackson.databind.ser.impl.UnknownSerializer.failForEmpty(UnknownSerializer.java:53)
	at com.fasterxml.jackson.databind.ser.impl.UnknownSerializer.serializeWithType(UnknownSerializer.java:40)
	at com.facebook.presto.metadata.AbstractTypedJacksonModule$InternalTypeSerializer.serialize(AbstractTypedJacksonModule.java:115)
	at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:732)
	at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:772)
	at com.fasterxml.jackson.databind.ser.BeanSerializer.serialize(BeanSerializer.java:178)
	at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:732)
	at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:772)
	at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeWithType(BeanSerializerBase.java:655)
	at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:734)
	at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:772)
	at com.fasterxml.jackson.databind.ser.BeanSerializer.serialize(BeanSerializer.java:178)
	at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider._serialize(DefaultSerializerProvider.java:479)
	at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:399)
	at com.fasterxml.jackson.databind.ObjectWriter$Prefetch.serialize(ObjectWriter.java:1568)
	at com.fasterxml.jackson.databind.ObjectWriter._writeValueAndClose(ObjectWriter.java:1273)
	at com.fasterxml.jackson.databind.ObjectWriter.writeValueAsBytes(ObjectWriter.java:1163)
	at com.facebook.airlift.json.JsonCodec.toJsonBytes(JsonCodec.java:214)
	... 7 more

Currently attempting

        @Override
        public ConnectorTransactionHandle beginTransaction(IsolationLevel isolationLevel, boolean readOnly)
        {
            return MockConnectorTransactionHandle.INSTANCE;
        }

Adding MockConnectorTransactionHandle from Trino.

This pushes more towards us having a complete separate Mock classes.

Stopping here for today.

mohsaka · 2025-03-13T17:14:17Z

Current issue

Caused by: com.fasterxml.jackson.databind.JsonMappingException: No connector for handle: INSTANCE of type class com.facebook.presto.connector.MockConnectorTransactionHandle (through reference chain: com.facebook.presto.sql.planner.PlanFragment["root"]->com.facebook.presto.spi.plan.TableScanNode["table"]->com.facebook.presto.spi.TableHandle["transaction"])
	at com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:402)
	at com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:361)
	at com.fasterxml.jackson.databind.ser.std.StdSerializer.wrapAndThrow(StdSerializer.java:323)
	at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:780)

Modified MockHandleResolver to use MockConnectorTransactionHandle.class

mohsaka · 2025-03-13T17:22:46Z

Caused by: java.lang.IllegalArgumentException: No connector for handle: INSTANCE of type class com.facebook.presto.testing.TestingHandle
	at com.facebook.presto.metadata.HandleResolver.getId(HandleResolver.java:228)
	at com.facebook.presto.metadata.HandleResolver.getId(HandleResolver.java:115)

Changed getTable to use TestingHandle.class

    @Override
    public Class<? extends ConnectorTableLayoutHandle> getTableLayoutHandleClass()
    {
        return TestingHandle.class;
    }

Also changed SplitClass

    @Override
    public Class<? extends ConnectorSplit> getSplitClass()
    {
        return MockConnectorFactory.MockConnector.MockConnectorSplit.class;
    }

mohsaka · 2025-03-13T18:19:25Z

Caused by: java.lang.ClassCastException: com.facebook.presto.connector.MockConnectorFactory$MockConnector$MockConnectorSplit cannot be cast to com.facebook.presto.tpch.TpchSplit
	at com.facebook.presto.tpch.TpchRecordSetProvider.getRecordSet(TpchRecordSetProvider.java:40)
	at com.facebook.presto.split.RecordPageSourceProvider.createPageSource(RecordPageSourceProvider.java:48)
	at com.facebook.presto.spi.connector.ConnectorPageSourceProvider.createPageSource(ConnectorPageSourceProvider.java:55)
	at com.facebook.presto.split.PageSourceManager.createPageSource(PageSourceManager.java:81)
	at com.facebook.presto.operator.TableScanOperator.getOutput(TableScanOperator.java:263)
	at com.facebook.presto.operator.Driver.processInternal(Driver.java:441)
	at com.facebook.presto.operator.Driver.lambda$processFor$10(Driver.java:324)
	at com.facebook.presto.operator.Driver.tryWithLock(Driver.java:750)
	at com.facebook.presto.operator.Driver.processFor(Driver.java:317)
	at com.facebook.presto.execution.SqlTaskExecution$DriverSplitRunner.processFor(SqlTaskExecution.java:1079)
	at com.facebook.presto.execution.executor.PrioritizedSplitRunner.process(PrioritizedSplitRunner.java:165)
	at com.facebook.presto.execution.executor.TaskExecutor$TaskRunner.run(TaskExecutor.java:621)
	at com.facebook.presto.$gen.Presto_null__testversion____20250313_180346_1.run(Unknown Source)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:750)

Currently trying to figure out what RecordSetProvider that trino uses for its mock tests.

mohsaka · 2025-03-13T22:22:36Z

Refactored. Current latest issue.

Caused by: java.lang.ClassCastException: com.facebook.presto.testing.TestingHandle cannot be cast to com.facebook.presto.connector.tvf.MockTableLayoutHandle
	at com.facebook.presto.connector.tvf.MockConnectorFactory$MockConnector$MockConnectorPageSourceProvider.createPageSource(MockConnectorFactory.java:353)
	at com.facebook.presto.split.PageSourceManager.createPageSource(PageSourceManager.java:81)
	at com.facebook.presto.operator.TableScanOperator.getOutput(TableScanOperator.java:263)
	at com.facebook.presto.operator.Driver.processInternal(Driver.java:441)
	at com.facebook.presto.operator.Driver.lambda$processFor$10(Driver.java:324)

mohsaka · 2025-03-14T00:47:24Z

Replaced Testing handle with MockTableLayout Handle. Now getting issues back at the planner stage.

Caused by: java.lang.IllegalArgumentException
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:129)
	at com.facebook.presto.metadata.TableLayoutResult.computeEnforced(TableLayoutResult.java:74)
	at com.facebook.presto.sql.planner.iterative.rule.PickTableLayout.pushPredicateIntoTableScan(PickTableLayout.java:328)
	at com.facebook.presto.sql.planner.iterative.rule.PickTableLayout.pushPredicateIntoTableScan(PickTableLayout.java:253)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges$Rewriter.planTableScan(AddExchanges.java:806)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges$Rewriter.visitTableScan(AddExchanges.java:738)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges$Rewriter.visitTableScan(AddExchanges.java:192)
	at com.facebook.presto.spi.plan.TableScanNode.accept(TableScanNode.java:213)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges$Rewriter.accept(AddExchanges.java:1565)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges$Rewriter.planChild(AddExchanges.java:1515)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges$Rewriter.visitOutput(AddExchanges.java:253)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges$Rewriter.visitOutput(AddExchanges.java:192)
	at com.facebook.presto.spi.plan.OutputNode.accept(OutputNode.java:98)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges$Rewriter.accept(AddExchanges.java:1565)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges$Rewriter.access$000(AddExchanges.java:192)
	at com.facebook.presto.sql.planner.optimizations.AddExchanges.optimize(AddExchanges.java:185)
	at com.facebook.presto.sql.planner.optimizations.StatsRecordingPlanOptimizer.optimize(StatsRecordingPlanOptimizer.java:58)
	at com.facebook.presto.sql.Optimizer.validateAndOptimizePlan(Optimizer.java:112)
	at com.facebook.presto.execution.SqlQueryExecution.lambda$doCreateLogicalPlanAndOptimize$5(SqlQueryExecution.java:586)
	at com.facebook.presto.common.RuntimeStats.recordWallAndCpuTime(RuntimeStats.java:158)
	at com.facebook.presto.execution.SqlQueryExecution.doCreateLogicalPlanAndOptimize(SqlQueryExecution.java:584)
	at com.facebook.presto.common.RuntimeStats.recordWallAndCpuTime(RuntimeStats.java:158)
	at com.facebook.presto.execution.SqlQueryExecution.createLogicalPlanAndOptimize(SqlQueryExecution.java:555)
	at com.facebook.presto.execution.SqlQueryExecution.start(SqlQueryExecution.java:483)
	at com.facebook.presto.$gen.Presto_null__testversion____20250314_004450_4.run(Unknown Source)

        // The engine requested the connector provides a layout with a non-none TupleDomain.
        // A TupleDomain is effectively a list of column-Domain pairs.
        // The connector is expected enforce the respective domain entirely on none, some, or all of the columns.
        // 1. When the connector could enforce none of the domains, the unenforced would be equal to predicate;
        // 2. When the connector could enforce some of the domains, the unenforced would contain a subset of the column-Domain pairs;
        // 3. When the connector could enforce all of the domains, the unenforced would be TupleDomain.all().

        // In all 3 cases shown above, the unenforced is not TupleDomain.none().
        checkArgument(!unenforced.isNone());

xin-zhang2 · 2025-03-14T13:43:58Z

@mohsaka The cause of the issue:
In MockConnectorFactory.getTableLayoutForConstraint, we pass TupleDomain.none() to set both predicate and unenforcedConstraint,

presto/presto-main/src/test/java/com/facebook/presto/connector/tvf/MockConnectorFactory.java

Lines 311 to 320 in 966505c

    
           public ConnectorTableLayoutResult getTableLayoutForConstraint( 
        
                   ConnectorSession session, 
        
                   ConnectorTableHandle table, 
        
                   Constraint<ColumnHandle> constraint, 
        
                   Optional<Set<ColumnHandle>> desiredColumns) 
        
           { 
        
               // TODO: Currently not supporting constraints 
        
               MockTableLayoutHandle mock = new MockTableLayoutHandle((MockConnectorTableHandle) table, TupleDomain.none()); 
        
               return new ConnectorTableLayoutResult(new ConnectorTableLayout(mock), TupleDomain.none()); 
        
           }

while new ConnectorTableLayout(mock) will re-set predicate in the layout to TupleDomain.all() due to

presto/presto-spi/src/main/java/com/facebook/presto/spi/ConnectorTableLayout.java

Lines 38 to 48 in 966505c

    
           public ConnectorTableLayout(ConnectorTableLayoutHandle handle) 
        
           { 
        
               this(handle, 
        
                       Optional.empty(), 
        
                       TupleDomain.all(), 
        
                       Optional.empty(), 
        
                       Optional.empty(), 
        
                       Optional.empty(), 
        
                       emptyList(), 
        
                       Optional.empty()); 
        
           }

When predicate is TupleDomain.all() and unenforcedConstraint is TupleDomain.none(), the following code will run into the current issue.

presto/presto-main/src/main/java/com/facebook/presto/metadata/TableLayoutResult.java

Lines 47 to 74 in 966505c

    
           public static TupleDomain<ColumnHandle> computeEnforced(TupleDomain<ColumnHandle> predicate, TupleDomain<ColumnHandle> unenforced) 
        
           { 
        
               if (predicate.isNone()) { 
        
                   // If the engine requests that the connector provides a layout with a domain of "none". The connector can have two possible reactions, either: 
        
                   // 1. The connector can provide an empty table layout. 
        
                   //   * There would be no unenforced predicate, i.e., unenforced predicate is TupleDomain.all(). 
        
                   //   * The predicate was successfully enforced. Enforced predicate would be same as predicate: TupleDomain.none(). 
        
                   // 2. The connector can't/won't. 
        
                   //   * The connector would tell the engine to put a filter on top of the scan, i.e., unenforced predicate is TupleDomain.none(). 
        
                   //   * The connector didn't successfully enforce anything. Therefore, enforced predicate would be TupleDomain.all(). 
        
                   if (unenforced.isNone()) { 
        
                       return TupleDomain.all(); 
        
                   } 
        
                   if (unenforced.isAll()) { 
        
                       return TupleDomain.none(); 
        
                   } 
        
                   throw new IllegalArgumentException(); 
        
               } 
        
               // The engine requested the connector provides a layout with a non-none TupleDomain. 
        
               // A TupleDomain is effectively a list of column-Domain pairs. 
        
               // The connector is expected enforce the respective domain entirely on none, some, or all of the columns. 
        
               // 1. When the connector could enforce none of the domains, the unenforced would be equal to predicate; 
        
               // 2. When the connector could enforce some of the domains, the unenforced would contain a subset of the column-Domain pairs; 
        
               // 3. When the connector could enforce all of the domains, the unenforced would be TupleDomain.all(). 
        
               // In all 3 cases shown above, the unenforced is not TupleDomain.none(). 
        
               checkArgument(!unenforced.isNone());

We can pass mock.getPredicate() to the constuctor of ConnectorTableLayout like

return new ConnectorTableLayoutResult(new ConnectorTableLayout(mock,
                        Optional.empty(),
                        mock.getPredicate(),
                        Optional.empty(),
                        Optional.empty(),
                        Optional.empty(),
                        Collections.emptyList(),
                        Optional.empty()), TupleDomain.none());

This change will get the testPrimitiveDefaultArgument passed, but I'm not sure if it is the correct way to solve this issue.
I will look deeper into the code.

mohsaka · 2025-03-14T17:07:20Z

@xin-zhang2 Nice, we have our first working test. Will push in that change and we can now work on the one with arguments.

Co-authored-by: Pratik Joseph Dabre <pdabre12@gmail.com> Co-authored-by: Xin Zhang <desertsxin@gmail.com>

mohsaka · 2025-03-14T17:21:41Z

First test is now clean, now going to the table function with arguments. The Layouts will have to be changed once we start moving into the statistics and partitioning.

Changes adapted from trino/PR#13106 Original commit: d0e2470c59f0de4be1e6320f1350b8806a72c548 Modifications were made to adapt to Presto including: Currently not formatting the arguments as we do not have a printer for Scalar Arguments.

mohsaka and others added 2 commits March 10, 2025 12:14

mohsaka force-pushed the tvf_12910 branch from 34dfb89 to db32775 Compare March 11, 2025 21:29

mohsaka force-pushed the tvf_12910 branch from db32775 to e642894 Compare March 12, 2025 19:34

mohsaka force-pushed the tvf_12910 branch 2 times, most recently from e633ced to d85e120 Compare March 13, 2025 00:29

mohsaka force-pushed the tvf_12910 branch from d85e120 to c872178 Compare March 13, 2025 02:23

mohsaka closed this Mar 13, 2025

mohsaka reopened this Mar 13, 2025

mohsaka force-pushed the tvf_12910 branch from c872178 to c897dc2 Compare March 13, 2025 18:09

Add and run first TVF connector tests

b0e6d34

Co-authored-by: Pratik Joseph Dabre <pdabre12@gmail.com> Co-authored-by: Xin Zhang <desertsxin@gmail.com>

mohsaka force-pushed the tvf_12910 branch from 966505c to b0e6d34 Compare March 14, 2025 17:19

Support TableFunctionNode in PlanPrinter

6377ece

Changes adapted from trino/PR#13106 Original commit: d0e2470c59f0de4be1e6320f1350b8806a72c548 Modifications were made to adapt to Presto including: Currently not formatting the arguments as we do not have a printer for Scalar Arguments.

mohsaka force-pushed the tvf_12910 branch from 21544f7 to 6377ece Compare March 14, 2025 19:58

mohsaka changed the title ~~Tvf 12910~~ Add the first connector test and fix all issues #3 Mar 14, 2025

Conversation

mohsaka commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

Uh oh!

mohsaka commented Mar 11, 2025

Uh oh!

mohsaka commented Mar 11, 2025

Uh oh!

mohsaka commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohsaka commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohsaka commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohsaka commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohsaka commented Mar 12, 2025

Uh oh!

mohsaka commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohsaka commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohsaka commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohsaka commented Mar 13, 2025

Uh oh!

mohsaka commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohsaka commented Mar 13, 2025

Uh oh!

mohsaka commented Mar 13, 2025

Uh oh!

mohsaka commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xin-zhang2 commented Mar 14, 2025

Uh oh!

mohsaka commented Mar 14, 2025

Uh oh!

mohsaka commented Mar 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mohsaka commented Mar 11, 2025 •

edited

Loading

mohsaka commented Mar 11, 2025 •

edited

Loading

mohsaka commented Mar 11, 2025 •

edited

Loading

mohsaka commented Mar 11, 2025 •

edited

Loading

mohsaka commented Mar 12, 2025 •

edited

Loading

mohsaka commented Mar 12, 2025 •

edited

Loading

mohsaka commented Mar 12, 2025 •

edited

Loading

mohsaka commented Mar 13, 2025 •

edited

Loading

mohsaka commented Mar 13, 2025 •

edited

Loading

mohsaka commented Mar 14, 2025 •

edited

Loading