Skip to content
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5582899
Init
anlowee Jul 21, 2025
e51aa97
Refactoring
anlowee Jul 21, 2025
14e61d2
Refactor
anlowee Jul 21, 2025
72cf371
Fix format check
anlowee Jul 21, 2025
1e5c256
Add docs
anlowee Jul 21, 2025
2079250
Merge branch 'release-0.293-clp-connector' into xwei/refactor-metadat…
anlowee Jul 21, 2025
bc6633e
Merge branch 'release-0.293-clp-connector' into xwei/refactor-metadat…
anlowee Jul 22, 2025
13caaa5
Apply suggestions from code review
anlowee Jul 29, 2025
df82f0f
Address comments
anlowee Jul 29, 2025
53fc10d
Merge branch 'release-0.293-clp-connector' into xwei/refactor-metadat…
anlowee Jul 29, 2025
36b2659
Update presto-clp/src/main/java/com/facebook/presto/plugin/clp/metada…
anlowee Jul 29, 2025
d639958
Address coderabbitai comments
anlowee Jul 29, 2025
190afeb
Merge branch 'xwei/refactor-metadatafilter-1' of github.com:anlowee/p…
anlowee Jul 29, 2025
aef56b4
Merge branch 'release-0.293-clp-connector' into xwei/refactor-metadat…
anlowee Jul 30, 2025
854401f
Fix the issue that when there are no filters in the query the require…
anlowee Jul 30, 2025
da18503
Merge branch 'xwei/refactor-metadatafilter-1' of github.com:anlowee/p…
anlowee Jul 30, 2025
eb70c70
Skip check for trieval queries
anlowee Jul 30, 2025
0a9f83e
Refactor the first parameter of checkContainsRequiredFilters to scope…
anlowee Jul 30, 2025
2e990ba
Make checkContainsRequiredFilters check for multiple table scopes and…
anlowee Jul 30, 2025
6586e25
Rename metadataDatabaseSpecific to metadataProviderSpecific
anlowee Jul 31, 2025
860b06f
Apply coderabbitai comments
anlowee Jul 31, 2025
dc6f9fc
Merge branch 'release-0.293-clp-connector' into xwei/refactor-metadat…
anlowee Aug 11, 2025
baf0053
Merge branch 'release-0.293-clp-connector' into xwei/refactor-metadat…
anlowee Aug 13, 2025
d1f34bf
Refactor docs.
kirkrodrigues Aug 14, 2025
c302fbe
Rename and address comments
anlowee Aug 14, 2025
6925011
Add a new config option clp.metadata-filter-provider-type
anlowee Aug 14, 2025
9289bd7
Renmae the metadata filter and metadata query to split filter and spl…
anlowee Aug 14, 2025
c72c0dc
Fix a format issue
anlowee Aug 15, 2025
bb68029
Update template config
anlowee Aug 15, 2025
4efe0b1
Address coderabbitai comment
anlowee Aug 15, 2025
528f5d0
Merge branch 'release-0.293-clp-connector' into xwei/refactor-metadat…
anlowee Aug 19, 2025
ee4ea9b
Address comments
anlowee Aug 20, 2025
5601349
Apply suggestions from code review
anlowee Aug 25, 2025
df37d2a
Renamed
anlowee Aug 25, 2025
1871733
Fix typo
anlowee Aug 25, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ public class ClpConfig
private String metadataDbName;
private String metadataDbUser;
private String metadataDbPassword;
private String metadataFilterConfig;
private String metadataTablePrefix;
private long metadataRefreshInterval = 60;
private long metadataExpireInterval = 600;

private String splitFilterConfig;
private SplitFilterProviderType splitFilterProviderType = SplitFilterProviderType.MYSQL;
private SplitProviderType splitProviderType = SplitProviderType.MYSQL;

public boolean isPolymorphicTypeEnabled()
Expand Down Expand Up @@ -108,18 +109,6 @@ public ClpConfig setMetadataDbPassword(String metadataDbPassword)
return this;
}

public String getMetadataFilterConfig()
{
return metadataFilterConfig;
}

@Config("clp.metadata-filter-config")
public ClpConfig setMetadataFilterConfig(String metadataFilterConfig)
{
this.metadataFilterConfig = metadataFilterConfig;
return this;
}

public String getMetadataTablePrefix()
{
return metadataTablePrefix;
Expand Down Expand Up @@ -162,6 +151,30 @@ public ClpConfig setMetadataExpireInterval(long metadataExpireInterval)
return this;
}

public String getSplitFilterConfig()
{
return splitFilterConfig;
}

@Config("clp.split-filter-config")
public ClpConfig setSplitFilterConfig(String splitFilterConfig)
{
this.splitFilterConfig = splitFilterConfig;
return this;
}
Comment on lines +159 to +164
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Add @ConfigDescription to document the new config key

Annotate the setter to improve self-documentation and config help output.

-    @Config("clp.split-filter-config")
+    @Config("clp.split-filter-config")
+    @com.facebook.airlift.configuration.ConfigDescription("Path to the split-filter configuration JSON file")
     public ClpConfig setSplitFilterConfig(String splitFilterConfig)

If you prefer an explicit import, add:

import com.facebook.airlift.configuration.ConfigDescription;
🤖 Prompt for AI Agents
In presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpConfig.java around
lines 159 to 164, the setter for splitFilterConfig is missing a
@ConfigDescription annotation; add @ConfigDescription("description of what
clp.split-filter-config controls") above the setSplitFilterConfig method (and
add the import com.facebook.airlift.configuration.ConfigDescription if not
already present) so the config key is documented in help output and
self-documentation.


public SplitFilterProviderType getSplitFilterProviderType()
{
return splitFilterProviderType;
}

@Config("clp.split-filter-provider-type")
public ClpConfig setSplitFilterProviderType(SplitFilterProviderType splitFilterProviderType)
{
this.splitFilterProviderType = splitFilterProviderType;
return this;
}
Comment on lines +171 to +176
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Add @ConfigDescription for provider type

Clarify what values are accepted and what it controls.

-    @Config("clp.split-filter-provider-type")
+    @Config("clp.split-filter-provider-type")
+    @com.facebook.airlift.configuration.ConfigDescription("Split-filter provider type (e.g., MYSQL)")
     public ClpConfig setSplitFilterProviderType(SplitFilterProviderType splitFilterProviderType)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Config("clp.split-filter-provider-type")
public ClpConfig setSplitFilterProviderType(SplitFilterProviderType splitFilterProviderType)
{
this.splitFilterProviderType = splitFilterProviderType;
return this;
}
@Config("clp.split-filter-provider-type")
@com.facebook.airlift.configuration.ConfigDescription("Split-filter provider type (e.g., MYSQL)")
public ClpConfig setSplitFilterProviderType(SplitFilterProviderType splitFilterProviderType)
{
this.splitFilterProviderType = splitFilterProviderType;
return this;
}


public SplitProviderType getSplitProviderType()
{
return splitProviderType;
Expand All @@ -179,6 +192,11 @@ public enum MetadataProviderType
MYSQL
}

public enum SplitFilterProviderType
{
MYSQL
}

public enum SplitProviderType
{
MYSQL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import com.facebook.airlift.bootstrap.LifeCycleManager;
import com.facebook.airlift.log.Logger;
import com.facebook.presto.plugin.clp.split.filter.ClpSplitFilterProvider;
import com.facebook.presto.spi.connector.Connector;
import com.facebook.presto.spi.connector.ConnectorMetadata;
import com.facebook.presto.spi.connector.ConnectorPlanOptimizerProvider;
Expand All @@ -40,7 +41,7 @@ public class ClpConnector
private final ClpSplitManager splitManager;
private final FunctionMetadataManager functionManager;
private final StandardFunctionResolution functionResolution;
private final ClpMetadataFilterProvider metadataFilterProvider;
private final ClpSplitFilterProvider splitFilterProvider;

@Inject
public ClpConnector(
Expand All @@ -50,21 +51,21 @@ public ClpConnector(
ClpSplitManager splitManager,
FunctionMetadataManager functionManager,
StandardFunctionResolution functionResolution,
ClpMetadataFilterProvider metadataFilterProvider)
ClpSplitFilterProvider splitFilterProvider)
{
this.lifeCycleManager = requireNonNull(lifeCycleManager, "lifeCycleManager is null");
this.metadata = requireNonNull(metadata, "metadata is null");
this.recordSetProvider = requireNonNull(recordSetProvider, "recordSetProvider is null");
this.splitManager = requireNonNull(splitManager, "splitManager is null");
this.functionManager = requireNonNull(functionManager, "functionManager is null");
this.functionResolution = requireNonNull(functionResolution, "functionResolution is null");
this.metadataFilterProvider = requireNonNull(metadataFilterProvider, "metadataFilterProvider is null");
this.splitFilterProvider = requireNonNull(splitFilterProvider, "splitFilterProvider is null");
}

@Override
public ConnectorPlanOptimizerProvider getConnectorPlanOptimizerProvider()
{
return new ClpPlanOptimizerProvider(functionManager, functionResolution, metadataFilterProvider);
return new ClpPlanOptimizerProvider(functionManager, functionResolution, splitFilterProvider);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ public enum ClpErrorCode
CLP_UNSUPPORTED_TYPE(3, EXTERNAL),
CLP_UNSUPPORTED_CONFIG_OPTION(4, EXTERNAL),

CLP_METADATA_FILTER_CONFIG_NOT_FOUND(10, USER_ERROR),
CLP_MANDATORY_METADATA_FILTER_NOT_VALID(11, USER_ERROR);
CLP_SPLIT_FILTER_CONFIG_NOT_FOUND(10, USER_ERROR),
CLP_MANDATORY_SPLIT_FILTER_NOT_VALID(11, USER_ERROR);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Renamed error codes align with split-filter refactor; verify all call sites updated

Names now match the split-filter terminology and keep the same numeric codes and types. Please ensure all throw sites and references have been migrated.

Run this quick search to catch any lingering references to the old constants or method names:


🏁 Script executed:

#!/bin/bash
# Find references to old metadata-filter error codes or method names
rg -n --fixed-strings \
  'CLP_METADATA_FILTER_CONFIG_NOT_FOUND|CLP_MANDATORY_METADATA_FILTER_NOT_VALID|remapFilterSql'

Length of output: 117


🏁 Script executed:

#!/bin/bash
echo "Search old constants and remapFilterSql..."
rg -n --hidden --no-ignore-vcs -S 'CLP_METADATA_FILTER_CONFIG_NOT_FOUND|CLP_MANDATORY_METADATA_FILTER_NOT_VALID|remapFilterSql' || echo "[no matches]"

echo "Search new constants (split)..."
rg -n --hidden --no-ignore-vcs -S 'CLP_SPLIT_FILTER_CONFIG_NOT_FOUND|CLP_MANDATORY_SPLIT_FILTER_NOT_VALID' || echo "[no matches]"

echo "Search generic split/metadata/filter strings..."
rg -n --hidden --no-ignore-vcs -S 'splitFilter|SplitFilter|split-filter|SPLIT_FILTER|METADATA_FILTER|metadata-filter' || echo "[no matches]"

echo "Search remapFilter*..."
rg -n --hidden --no-ignore-vcs -S 'remapFilter|remap.*Filter' || echo "[no matches]"

echo "Search ClpErrorCode usages..."
rg -n --hidden --no-ignore-vcs -S 'ClpErrorCode' || echo "[no matches]"

echo "Print ClpErrorCode.java (if present)..."
if [ -f presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpErrorCode.java ]; then
  sed -n '1,240p' presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpErrorCode.java
else
  echo "[file not found: presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpErrorCode.java]"
fi

Length of output: 300579


Split-filter rename verified — code updated; docs contain lingering metadata-filter-config

Checked the repository: old constants/method names (CLP_METADATA_FILTER_CONFIG_NOT_FOUND, CLP_MANDATORY_METADATA_FILTER_NOT_VALID, remapFilterSql) are not present; the new split-filter constants and methods are used.

Verified usages (non-exhaustive):

  • presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpErrorCode.java — CLP_SPLIT_FILTER_CONFIG_NOT_FOUND, CLP_MANDATORY_SPLIT_FILTER_NOT_VALID
  • presto-clp/src/main/java/com/facebook/presto/plugin/clp/split/filter/ClpSplitFilterProvider.java — imports and throws using the new CLP_SPLIT_* codes
  • presto-clp/src/main/java/com/facebook/presto/plugin/clp/split/filter/ClpMySqlSplitFilterProvider.java — remapSplitFilterPushDownExpression implementation
  • presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizer.java — calls splitFilterProvider.remapSplitFilterPushDownExpression(...)
  • presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpConfig.java — clp.split-filter-config property and SplitFilterProviderType
  • Tests: presto-clp/src/test/java/com/facebook/presto/plugin/clp/split/filter/TestClpMySqlSplitFilterConfig*.java — exercise remapSplitFilterPushDownExpression

Needs attention:

  • presto-docs/src/main/sphinx/connector/clp.rst: line ~33 still references clp.metadata-filter-config=/path/to/metadata-filter-config.json — update documentation to use the new clp.split-filter-config (or otherwise reconcile the naming) so docs match the refactor.
🤖 Prompt for AI Agents
In presto-docs/src/main/sphinx/connector/clp.rst around line ~33, the docs still
reference the old property name clp.metadata-filter-config; update all
occurrences to the new property clp.split-filter-config (including example usage
clp.split-filter-config=/path/to/split-filter-config.json), update any
explanatory text to use "split filter" terminology (and the corresponding file
name in examples), and run a quick repo-wide search/update for any remaining
"metadata-filter-config" references in the docs to ensure consistency with the
refactor.


private final ErrorCode errorCode;

Expand Down

This file was deleted.

Loading
Loading