feat: Add taskfiles for coordinator-side of Presto CLP connector.#7
feat: Add taskfiles for coordinator-side of Presto CLP connector.#720001020ycx wants to merge 10 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 54 minutes and 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces code formatting and linting infrastructure to the Presto CLP connector. It adds Spotless and SpotBugs Maven plugins, an Eclipse formatter configuration, SpotBugs exclusion patterns, documentation, and task automation. The majority of changes apply consistent formatting to all Java source files without altering functionality. ChangesCode Formatting and Linting Tool Setup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
presto-connector/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpComputePushDown.java (1)
76-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize negation style in conditionals to match the Java guideline (
false == <expression>).
- Line 76: change
!rewriter.tableScopeSet.isEmpty()and!rewriter.hasVisitedFilter.- Line 103: change
!(node.getSource() instanceof TableScanNode).- Lines 138-139: change
!metadataSqlQuery.get().isEmpty().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@presto-connector/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpComputePushDown.java` at line 76, The conditionals use prefix negation (!) instead of the project's preferred Java style (Yoda-style boolean comparison); replace occurrences like !rewriter.tableScopeSet.isEmpty() and !rewriter.hasVisitedFilter with false == rewriter.tableScopeSet.isEmpty() and false == rewriter.hasVisitedFilter, change !(node.getSource() instanceof TableScanNode) to false == (node.getSource() instanceof TableScanNode), and replace !metadataSqlQuery.get().isEmpty() with false == metadataSqlQuery.get().isEmpty() so all four identified expressions (rewriter.tableScopeSet.isEmpty, rewriter.hasVisitedFilter, node.getSource() instanceof TableScanNode, metadataSqlQuery.get().isEmpty) use the normalized negation style.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@presto-connector/README.md`:
- Line 40: The README line "To fix fixable issues and run static analysis:"
contains a duplicated word; update the sentence in the README (the heading/line
that currently starts "To fix fixable issues and run static analysis:") to
remove the extra "fix" so it reads "To fix fixable issues and run static
analysis:" corrected to "To fix fixable issues and run static analysis:" (remove
the duplicate) — simply edit that line in README.md to "To fix fixable issues
and run static analysis:".
In `@presto-connector/spotbugs-exclude.xml`:
- Around line 14-28: The current broad SpotBugs exclusions in
spotbugs-exclude.xml mask real issues; either fix the underlying bugs in the
referenced classes (handle thrown exceptions in constructors and close DB
resources in ClpMySqlMetadataProvider and ClpMySqlSplitProvider, and stop
exposing internal mutable state in classes under
com.facebook.presto.plugin.clp.mockdb) or replace each <Match> block with a
per-instance suppression that names the exact class+method/field and includes a
one-line justification and a tracking ticket ID; specifically address patterns
CT_CONSTRUCTOR_THROW and ODR_OPEN_DATABASE_RESOURCE for ClpMySqlMetadataProvider
and split.ClpMySqlSplitProvider, CT_CONSTRUCTOR_THROW for
split.filter.ClpSplitFilterProvider, and EI_EXPOSE_REP/EI_EXPOSE_REP2 for
com.facebook.presto.plugin.clp.mockdb.*, and remove any broad regex matches once
concrete fixes or narrow, justified suppressions exist.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpConfig.java`:
- Around line 89-92: Update the validation condition in ClpConfig to use
guideline-compliant boolean comparison: replace the unary-negation form used on
SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix).matches() with an
explicit equality comparison (false == ...matches()). Specifically adjust the if
condition that checks metadataTablePrefix and SAFE_SQL_TABLE_NAME_PATTERN so it
reads the equivalent of "metadataTablePrefix == null || false ==
SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix).matches()" to satisfy
the repo rule; keep the null check for metadataTablePrefix and preserve existing
behavior.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpMetadata.java`:
- Around line 89-92: In listTables(ConnectorSession session, Optional<String>
schemaName) replace the negation guard that uses
"!listSchemaNames(session).contains(schemaNameValue)" with an explicit
comparison using "false == listSchemaNames(session).contains(schemaNameValue)";
similarly find the other guard that currently uses a leading '!' (the one
calling listSchemaNames(session) around the same area/second clause) and change
it to "false == <expression>" so both negations use the explicit false == form
per repository rule; ensure you update both occurrences while leaving the
surrounding logic and returns unchanged.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java`:
- Line 48: The type-check guard in ClpColumnHandleCodec uses the negation form
`!(handle instanceof ClpColumnHandle)`; per repo guidelines replace the negation
with the preferred boolean style by comparing to false, i.e., change the
condition to use `false == (handle instanceof ClpColumnHandle)` so the check
still rejects non-ClpColumnHandle instances while conforming to the coding
standard; update the if condition in ClpColumnHandleCodec accordingly
referencing the ClpColumnHandle type.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpSplitCodec.java`:
- Line 40: Replace the negated instanceof check in ClpSplitCodec: change the
condition that currently reads "if (!(handle instanceof ClpSplit))" to use the
project boolean style "false == (handle instanceof ClpSplit)"; update the if
condition around the handle variable and ClpSplit type check accordingly so the
logic and early-return/error behavior remains identical.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpTableHandleCodec.java`:
- Line 39: The type-check in ClpTableHandleCodec currently uses a negation form
"if (!(handle instanceof ClpTableHandle))"; change it to follow the repo boolean
style by replacing the condition with the equivalent equality form, e.g., "if
(false == (handle instanceof ClpTableHandle))" in the method where the check
appears so the semantics are unchanged but the boolean comparison rule is
honored.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpTableLayoutHandleCodec.java`:
- Line 44: Update the boolean-style guard in ClpTableLayoutHandleCodec so it
uses the project convention `false == <expr>` instead of `!<expr>`: replace the
`if (!(handle instanceof ClpTableLayoutHandle))` check with `if (false ==
(handle instanceof ClpTableLayoutHandle))`, preserving the existing logic and
surrounding code (the check inside ClpTableLayoutHandleCodec where the handle
type is validated).
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpMySqlMetadataProvider.java`:
- Around line 110-113: In ClpMySqlMetadataProvider, update the negation style to
match project convention: replace uses of the ! operator with explicit
comparison to false—for example change !archiveStorageDirectory.isEmpty() to
false == archiveStorageDirectory.isEmpty() in the conditional that checks
isValidIdentifier(tableName) and archiveStorageDirectory, and likewise change
!dbName.isEmpty() to false == dbName.isEmpty() where dbName != null is also
checked; preserve existing null checks and overall conditional logic when making
these substitutions.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTree.java`:
- Around line 132-133: In resolvePolymorphicConflicts(ClpNode parent, String
baseName, Type newType) replace the direct boolean negations with the
guideline-compliant equality form: change the check using polymorphicTypeEnabled
(currently written as if (!polymorphicTypeEnabled)) to use if (false ==
polymorphicTypeEnabled), and change the equals negation in the conflict branch
(currently written as else if (!existing.type.equals(newType))) to else if
(false == existing.type.equals(newType)); leave all other logic unchanged and
ensure imports and formatting remain consistent.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpFilterToKqlConverter.java`:
- Around line 315-318: The code builds a negated ClpExpression using
expression.getMetadataSqlQuery() which is an Optional, causing "Optional[...]"
to appear; update the construction in ClpFilterToKqlConverter to extract the
wrapped SQL string (e.g., expression.getMetadataSqlQuery().orElse("") or
orElseThrow if absence is an error) and use that raw value when concatenating
("NOT " + <rawSql>), keeping the existing notPushDownExpression and returning
new ClpExpression(...) as before.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/split/filter/ClpMySqlSplitFilterProvider.java`:
- Around line 138-141: In the equals method of RangeMapping, replace the negated
instanceof check currently written as "!(o instanceof RangeMapping)" with the
guideline-compliant form using equality-to-false (e.g., "false == (o instanceof
RangeMapping)"); update the conditional in the equals(Object o) method so it
uses that form and keeps the same control flow and casting to RangeMapping.
In
`@presto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.java`:
- Around line 71-73: The current hasType in TestClpConnectorCodecProvider's
TypeManager delegates to getType and can propagate IllegalArgumentException for
unknown TypeSignature bases; change hasType to be non-throwing by either (a)
inspecting the TypeSignature base (signature.getBase()) and returning true only
for the known bases your getType supports, or (b) wrapping the getType call in a
try/catch and returning false on IllegalArgumentException; update the hasType
method (not getType) so callers get a safe boolean result without exceptions.
---
Outside diff comments:
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpComputePushDown.java`:
- Line 76: The conditionals use prefix negation (!) instead of the project's
preferred Java style (Yoda-style boolean comparison); replace occurrences like
!rewriter.tableScopeSet.isEmpty() and !rewriter.hasVisitedFilter with false ==
rewriter.tableScopeSet.isEmpty() and false == rewriter.hasVisitedFilter, change
!(node.getSource() instanceof TableScanNode) to false == (node.getSource()
instanceof TableScanNode), and replace !metadataSqlQuery.get().isEmpty() with
false == metadataSqlQuery.get().isEmpty() so all four identified expressions
(rewriter.tableScopeSet.isEmpty, rewriter.hasVisitedFilter, node.getSource()
instanceof TableScanNode, metadataSqlQuery.get().isEmpty) use the normalized
negation style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b9036d80-0f85-40e0-b4cc-7f384ff8491f
📒 Files selected for processing (58)
presto-connector/.eclipse-formatter.xmlpresto-connector/README.mdpresto-connector/lint-tasks.yamlpresto-connector/pom.xmlpresto-connector/spotbugs-exclude.xmlpresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpColumnHandle.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpConfig.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpConnector.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpConnectorFactory.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpErrorCode.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpExpression.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpFunctions.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpHandleResolver.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpMetadata.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpModule.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpPlugin.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpRecordSetProvider.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpSplit.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpSplitManager.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpTableHandle.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpTableLayoutHandle.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpTransactionHandle.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpConnectorCodecProvider.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpSplitCodec.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpTableHandleCodec.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpTableLayoutHandleCodec.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpTransactionHandleCodec.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/CodecUtils.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpMetadataProvider.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpMySqlMetadataProvider.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTree.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTreeNodeType.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpComputePushDown.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpFilterToKqlConverter.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpPlanOptimizerProvider.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpUdfRewriter.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/split/ClpMySqlSplitProvider.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/split/ClpSplitProvider.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/split/filter/ClpMySqlSplitFilterProvider.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/split/filter/ClpSplitFilterConfig.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/split/filter/ClpSplitFilterConfigCustomOptionsDeserializer.javapresto-connector/src/main/java/com/facebook/presto/plugin/clp/split/filter/ClpSplitFilterProvider.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/ClpMetadataDbSetUp.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/ClpQueryRunner.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/TestClpMetadata.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/TestClpSplit.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/TestClpUdfRewriter.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/mockdb/ClpMockMetadataDatabase.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/mockdb/table/ArchivesTableRows.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/mockdb/table/ColumnMetadataTableRows.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/mockdb/table/DatasetsTableRows.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/split/filter/TestClpMySqlSplitFilterConfig.javapresto-connector/src/test/java/com/facebook/presto/plugin/clp/split/filter/TestClpSplitFilterConfigCommon.javapresto-connector/taskfile.yaml
| if ( | ||
| metadataTablePrefix == null || !SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix) | ||
| .matches() | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use guideline-compliant boolean comparison in the validation condition.
Line 90 currently uses unary negation. Please switch to false == ...matches() to match the repo rule.
Proposed change
- metadataTablePrefix == null || !SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix)
+ metadataTablePrefix == null || false == SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix)
.matches()As per coding guidelines **/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
📝 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.
| if ( | |
| metadataTablePrefix == null || !SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix) | |
| .matches() | |
| ) { | |
| if ( | |
| metadataTablePrefix == null || false == SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix) | |
| .matches() | |
| ) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@presto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpConfig.java`
around lines 89 - 92, Update the validation condition in ClpConfig to use
guideline-compliant boolean comparison: replace the unary-negation form used on
SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix).matches() with an
explicit equality comparison (false == ...matches()). Specifically adjust the if
condition that checks metadataTablePrefix and SAFE_SQL_TABLE_NAME_PATTERN so it
reads the equivalent of "metadataTablePrefix == null || false ==
SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix).matches()" to satisfy
the repo rule; keep the null check for metadataTablePrefix and preserve existing
behavior.
| public List<SchemaTableName> listTables(ConnectorSession session, Optional<String> schemaName) { | ||
| String schemaNameValue = schemaName.orElse(DEFAULT_SCHEMA_NAME); | ||
| if (!listSchemaNames(session).contains(schemaNameValue)) { | ||
| return ImmutableList.of(); | ||
| } | ||
|
|
||
| return listTables(schemaNameValue).stream() | ||
| .map(tableHandle -> new SchemaTableName(schemaNameValue, tableHandle.getSchemaTableName().getTableName())) | ||
| .collect(toImmutableList()); | ||
| if (!listSchemaNames(session).contains(schemaNameValue)) { return ImmutableList.of(); } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace negation checks with explicit false == ... per repository rule.
Use explicit comparison instead of ! in both guard clauses at Line 91 and Line 107.
Suggested patch
- if (!listSchemaNames(session).contains(schemaNameValue)) { return ImmutableList.of(); }
+ if (false == listSchemaNames(session).contains(schemaNameValue)) { return ImmutableList.of(); }
@@
- if (!listSchemaNames(session).contains(schemaName)) { return null; }
+ if (false == listSchemaNames(session).contains(schemaName)) { return null; }As per coding guidelines, "**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}: - Prefer false == <expression> rather than !<expression>."
Also applies to: 102-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/ClpMetadata.java`
around lines 89 - 92, In listTables(ConnectorSession session, Optional<String>
schemaName) replace the negation guard that uses
"!listSchemaNames(session).contains(schemaNameValue)" with an explicit
comparison using "false == listSchemaNames(session).contains(schemaNameValue)";
similarly find the other guard that currently uses a leading '!' (the one
calling listSchemaNames(session) around the same area/second clause) and change
it to "false == <expression>" so both negations use the explicit false == form
per repository rule; ensure you update both occurrences while leaving the
surrounding logic and returns unchanged.
| { | ||
| public byte[] serialize(ColumnHandle handle) { | ||
| try { | ||
| if (!(handle instanceof ClpColumnHandle)) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use guideline-compliant boolean style for the type guard.
Line 48 uses !<expression>; this should be rewritten to false == <expression> per repo rules.
Proposed change
- if (!(handle instanceof ClpColumnHandle)) {
+ if (false == (handle instanceof ClpColumnHandle)) {As per coding guidelines, "Prefer false == <expression> rather than !<expression>."
📝 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.
| if (!(handle instanceof ClpColumnHandle)) { | |
| if (false == (handle instanceof ClpColumnHandle)) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java`
at line 48, The type-check guard in ClpColumnHandleCodec uses the negation form
`!(handle instanceof ClpColumnHandle)`; per repo guidelines replace the negation
with the preferred boolean style by comparing to false, i.e., change the
condition to use `false == (handle instanceof ClpColumnHandle)` so the check
still rejects non-ClpColumnHandle instances while conforming to the coding
standard; update the if condition in ClpColumnHandleCodec accordingly
referencing the ClpColumnHandle type.
| if ( | ||
| isValidIdentifier(tableName) && archiveStorageDirectory != null | ||
| && !archiveStorageDirectory.isEmpty() | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="presto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpMySqlMetadataProvider.java"
# Print the relevant regions with line numbers
nl -ba "$file" | sed -n '90,130p'
echo "----"
nl -ba "$file" | sed -n '130,170p'Repository: y-scope/clp-plugin-presto-connector
Length of output: 122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="presto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpMySqlMetadataProvider.java"
awk -v start=95 -v end=130 'NR>=start && NR<=end {printf "%d\t%s\n", NR, $0}' "$file"
echo "----"
awk -v start=130 -v end=160 'NR>=start && NR<=end {printf "%d\t%s\n", NR, $0}' "$file"Repository: y-scope/clp-plugin-presto-connector
Length of output: 2581
Align negation style with project rule (false == <expr> instead of !<expr>).
- In
presto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpMySqlMetadataProvider.java(around lines 110-113), replace!archiveStorageDirectory.isEmpty()withfalse == archiveStorageDirectory.isEmpty(). - In the same file (around line 137), replace
!dbName.isEmpty()withfalse == dbName.isEmpty()(keeping thedbName != nullcheck).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpMySqlMetadataProvider.java`
around lines 110 - 113, In ClpMySqlMetadataProvider, update the negation style
to match project convention: replace uses of the ! operator with explicit
comparison to false—for example change !archiveStorageDirectory.isEmpty() to
false == archiveStorageDirectory.isEmpty() in the conditional that checks
isValidIdentifier(tableName) and archiveStorageDirectory, and likewise change
!dbName.isEmpty() to false == dbName.isEmpty() where dbName != null is also
checked; preserve existing null checks and overall conditional logic when making
these substitutions.
| private String resolvePolymorphicConflicts(ClpNode parent, String baseName, Type newType) { | ||
| if (!polymorphicTypeEnabled) { return baseName; } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="presto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTree.java"
# Print relevant section with line numbers
nl -ba "$FILE" | sed -n '110,170p'Repository: y-scope/clp-plugin-presto-connector
Length of output: 122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="presto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTree.java"
# Show around the referenced lines with available line-numbering
cat -n "$FILE" | sed -n '120,170p'Repository: y-scope/clp-plugin-presto-connector
Length of output: 2382
Use guideline-compliant boolean negation style (false == <expression>).
- Line 133: replace
if (!polymorphicTypeEnabled)withif (false == polymorphicTypeEnabled). - Line 140: replace
else if (!existing.type.equals(newType))withelse if (false == existing.type.equals(newType)).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTree.java`
around lines 132 - 133, In resolvePolymorphicConflicts(ClpNode parent, String
baseName, Type newType) replace the direct boolean negations with the
guideline-compliant equality form: change the check using polymorphicTypeEnabled
(currently written as if (!polymorphicTypeEnabled)) to use if (false ==
polymorphicTypeEnabled), and change the equals negation in the conflict branch
(currently written as else if (!existing.type.equals(newType))) to else if
(false == existing.type.equals(newType)); leave all other logic unchanged and
ensure imports and formatting remain consistent.
| return new ClpExpression( | ||
| notPushDownExpression, | ||
| "NOT " + expression.getMetadataSqlQuery() | ||
| ); |
There was a problem hiding this comment.
Metadata SQL negation uses Optional wrapper text instead of raw SQL.
At Line 317, concatenating expression.getMetadataSqlQuery() yields Optional[...] in the SQL text. Use the contained value.
Proposed fix
if (expression.getMetadataSqlQuery().isPresent()) {
return new ClpExpression(
notPushDownExpression,
- "NOT " + expression.getMetadataSqlQuery()
+ "NOT " + expression.getMetadataSqlQuery().get()
);
} else {📝 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.
| return new ClpExpression( | |
| notPushDownExpression, | |
| "NOT " + expression.getMetadataSqlQuery() | |
| ); | |
| return new ClpExpression( | |
| notPushDownExpression, | |
| "NOT " + expression.getMetadataSqlQuery().get() | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpFilterToKqlConverter.java`
around lines 315 - 318, The code builds a negated ClpExpression using
expression.getMetadataSqlQuery() which is an Optional, causing "Optional[...]"
to appear; update the construction in ClpFilterToKqlConverter to extract the
wrapped SQL string (e.g., expression.getMetadataSqlQuery().orElse("") or
orElseThrow if absence is an error) and use that raw value when concatenating
("NOT " + <rawSql>), keeping the existing notPushDownExpression and returning
new ClpExpression(...) as before.
| public boolean equals(Object o) { | ||
| if (this == o) { return true; } | ||
| if (!(o instanceof RangeMapping)) { return false; } | ||
| RangeMapping that = (RangeMapping)o; |
There was a problem hiding this comment.
Use guideline-compliant boolean negation style in equals.
Line 140 uses !(o instanceof RangeMapping). Please rewrite this using the preferred false == <expression> form.
Suggested patch
- if (!(o instanceof RangeMapping)) { return false; }
+ if (false == (o instanceof RangeMapping)) { return false; }As per coding guidelines, "Prefer false == <expression> rather than !<expression>."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@presto-connector/src/main/java/com/facebook/presto/plugin/clp/split/filter/ClpMySqlSplitFilterProvider.java`
around lines 138 - 141, In the equals method of RangeMapping, replace the
negated instanceof check currently written as "!(o instanceof RangeMapping)"
with the guideline-compliant form using equality-to-false (e.g., "false == (o
instanceof RangeMapping)"); update the conditional in the equals(Object o)
method so it uses that form and keeps the same control flow and casting to
RangeMapping.
| public boolean hasType(TypeSignature signature) { | ||
| return getType(signature) != null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Presto 0.297.0 TypeManager API, is hasType(TypeSignature) expected to return false (not throw) for unknown types?
💡 Result:
Expected behavior: yes—TypeManager.hasType(TypeSignature) is expected to return false (and not throw) when the TypeSignature is unknown. In Presto’s TypeManager implementation, type lookup for an exact TypeSignature returns null when the type isn’t registered, and callers can treat that as “not present” rather than as an exceptional condition. Specifically, the implementation gets the Type from an internal map/cache and only returns a Type when it’s non-null; otherwise it returns null rather than throwing [1].
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section around lines 71-73
sed -n '1,140p' presto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.java | nl -ba | sed -n '55,110p'
# Locate getType declarations/overrides in this file
rg -n "getType\\(" presto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.java
# Show the immediate implementation(s) of getType/hasType in this file
rg -n "public\\s+.*hasType\\(" presto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.java
rg -n "hasType\\(" presto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.java
# If getType is inherited, find the class/interface hierarchy used by this provider
# Print the file header with class declaration/imports
sed -n '1,120p' presto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.java | nl -ba | head -n 120Repository: y-scope/clp-plugin-presto-connector
Length of output: 122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="presto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.java"
# Print the relevant region with line numbers via awk
sed -n '55,100p' "$FILE" | awk '{printf "%d\t%s\n", NR+54, $0}'
# Find hasType/getType references in this file
rg -n "hasType\\(" "$FILE" || true
rg -n "getType\\(" "$FILE" || true
# Show class declaration section (imports + class header)
sed -n '1,120p' "$FILE" | awk '{printf "%d\t%s\n", NR, $0}'
# If this file references a parent/provider type, capture the class name(s) and search for getType implementation elsewhere
# (First list "extends" / "implements" in this file)
rg -n "class\\s+\\w+\\s*(extends|implements)" "$FILE" || trueRepository: y-scope/clp-plugin-presto-connector
Length of output: 7288
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for hasType usage in this repo
rg -n "hasType\\(" presto-connector/src/test/java presto-connector/src/main/java presto-connector/src -S || true
# If TypeManager interface is vendored in this repo, find it
rg -n "interface\\s+TypeManager|class\\s+TypeManager" -S . || trueRepository: y-scope/clp-plugin-presto-connector
Length of output: 407
🌐 Web query:
Presto 0.297.0 TypeManager hasType TypeSignature javadoc returns false unknown
💡 Result:
I couldn’t find any authoritative Presto 0.297.0 source/javadoc that contains a TypeManager method named hasType(TypeSignature) returning false; the only concrete evidence I found is a TypeManager interface in other Presto forks/changes that doesn’t define such a hasType method. What I did find that is relevant to your “TypeManager + TypeSignature” wording: 1) Presto’s public Type system docs describe that a “type signature” is used to define type identity and (for parametric types) encodes type parameters/literal parameters, and that plugins can provide new Type objects via getTypes and getParametricTypes [1]. 2) A Presto fork commit introducing changes to com.facebook.presto.common.type.TypeManager shows methods like canCoerce, getParameterizedType, and various parametric-type related APIs, but not hasType(TypeSignature) [2]. 3) The query you referenced also mentions “unknown” and “Presto 0.297.0 TypeManager … javadoc”; none of the search results surfaced a javadoc page for TypeManager 0.297.0 with a hasType(TypeSignature) method. If you can share the exact exception/stack trace text (or the interface/class name where hasType is called) and the exact Presto artifact coordinates you’re using (presto version and jar groupId/artifactId), I can re-search for the precise method signature and the “unknown” behavior you’re seeing.
Citations:
🌐 Web query:
"TypeManager" "boolean hasType" "TypeSignature" "returns false" presto
💡 Result:
I couldn’t find any Presto source (in the results I located) that contains the exact code snippet you’re searching for: TypeManager + “boolean hasType” + TypeSignature + “returns false”. What I did find instead is: 1) Presto defines a TypeManager interface used to work with types and type signatures, including methods like getType(TypeSignature) and getParameterizedType(...), plus coercion support. Evidence: the “Add a native type manager” commit shows TypeManager’s methods such as canCoerce(...) and getParameterizedType(...), and an implementation of getType(TypeSignature) on the TypeManager side. [1][2] 2) The “Add a native type manager” Presto PR discusses a “native type manager” addition to let the coordinator know what types the C++ native execution engine supports; it specifically mentions that CHAR isn’t supported by the native engine and that queries/CREATE TABLE with CHAR would fail because CHAR would not be exposed as a type. This explains a “returns false” behavior in practice (unknown/unsupported types), but not the exact “hasType(... TypeSignature) returns false” API you requested. [3] 3) Presto’s Types documentation explains the role of type signatures in identifying types and how plugins provide types (via getTypes / getParametricTypes). This supports the idea that unsupported types may not be exposed/available through the TypeManager, but again doesn’t show the specific hasType(...) method you cited. [4] If you can paste the exact file/class where you saw “TypeManager boolean hasType TypeSignature returns false” (or provide the method signature text around it), I can do a more targeted search for that exact method body and explain why it returns false in Presto.
Citations:
- 1: shantanusharma/presto@68cdacd
- 2: sadroeck/presto@68cdacd
- 3: Add a native type manager prestodb/presto#24179
- 4: https://prestodb.io/docs/current/develop/types.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for hasType usage in this repo
rg -n "hasType\\(" presto-connector/src/test/java presto-connector/src/main/java presto-connector/src -S || true
# If TypeManager interface is vendored in this repo, find it
rg -n "interface\\s+TypeManager|class\\s+TypeManager" -S . || trueRepository: y-scope/clp-plugin-presto-connector
Length of output: 407
🌐 Web query:
Presto 0.297.0 TypeManager hasType TypeSignature javadoc
💡 Result:
No: I could not find Javadoc for a Presto 0.297.0 class/interface named TypeManager that documents a method hasType(TypeSignature...). What I found instead (grounded in source): 1) In the Presto codebase, TypeManager is declared in presto-common at com.facebook.presto.common.type.TypeManager.java, but the authoritative snippet I located shows methods like getParameterizedType(...), canCoerce(...), and (in one branch/commit) instantiateParametricType(...), getTypes, etc.—not a hasType(...) method for TypeSignature. [1][2] 2) The official Presto docs pages I located for 0.297 focus on data types generally and do not include any TypeManager.hasType(TypeSignature) Javadoc entry. [3] If you can share a link to the specific Javadoc page you expect (or the exact Maven artifact/version that contains “Presto 0.297.0 TypeManager”), I can re-check precisely whether such a hasType(...) method exists there (it may have a different name/signature, or it may belong to a different manager/registry interface than TypeManager).
Citations:
- 1: shantanusharma/presto@68cdacd
- 2: sadroeck/presto@68cdacd
- 3: https://prestodb.io/docs/current/language/types.html
🌐 Web query:
TypeManager boolean hasType(TypeSignature) unknown type returns false
💡 Result:
I couldn’t find any authoritative source for a method literally named TypeManager.hasType(TypeSignature) with the behavior “unknown type returns false”. What I did find is that “TypeManager” exists in multiple unrelated ecosystems, and their APIs differ: 1) Unity DOTS (Entities) has Unity.Entities.TypeManager, but its API is different (e.g., it includes methods like GetTypeIndex(Type)/GetType(TypeIndex) rather than hasType(TypeSignature)). [1] 2) A Java “TypeManager” implementation in DataNucleus is unrelated and provides different methods (e.g., converter registration, JavaType resolution), not hasType(TypeSignature). [2] 3) In Trino/Presto, there is a TypeManager concept for type resolution, but the issue discussions show using typeManager.getType(new TypeSignature(...)) rather than a hasType(...) boolean check; failures were about type registration in test environments. [3][4] If you tell me which platform/library you mean by “TypeManager” (e.g., Unity.Entities, Trino/Presto, DataNucleus, Soot, something else) and provide the exact interface/class name (or a link to the code), I can re-research precisely and confirm whether an “unknown type => false” boolean method exists and where it’s specified.
Citations:
- 1: https://docs.unity3d.com/Packages/com.unity.entities@1.2/api/Unity.Entities.TypeManager.html
- 2: https://www.datanucleus.org/javadocs/core/5.0/org/datanucleus/store/types/TypeManagerImpl.html
- 3: GeometryType doesn't work similar with SPI Types trinodb/trino#9845
- 4: Add WKB Support for PostGIS Geometry Columns trinodb/trino#5580
Make hasType non-throwing for unknown TypeSignatures
In TestClpConnectorCodecProvider’s TypeManager implementation, hasType delegates to getType, and getType throws IllegalArgumentException for unknown type bases—so this boolean probe can unexpectedly throw instead of returning false.
File: presto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.java
Lines: 71-73
Suggested fix
`@Override`
public boolean hasType(TypeSignature signature) {
- return getType(signature) != null;
+ try {
+ return getType(signature) != null;
+ }
+ catch (IllegalArgumentException e) {
+ return false;
+ }
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@presto-connector/src/test/java/com/facebook/presto/plugin/clp/codec/TestClpConnectorCodecProvider.java`
around lines 71 - 73, The current hasType in TestClpConnectorCodecProvider's
TypeManager delegates to getType and can propagate IllegalArgumentException for
unknown TypeSignature bases; change hasType to be non-throwing by either (a)
inspecting the TypeSignature base (signature.getBase()) and returning true only
for the known bases your getType supports, or (b) wrapping the getType call in a
try/catch and returning false on IllegalArgumentException; update the hasType
method (not getType) so callers get a safe boolean result without exceptions.
20001020ycx
force-pushed
the
feat/2026-05-21-presto-connector-taskfile
branch
from
b96d3a9 to
148ce52
Compare
- Add .eclipse-formatter.xml for Spotless Java formatting - Add spotbugs-exclude.xml with proper pattern-based excludes - Add lint-tasks.yaml and taskfile.yaml for running lint/format - Add README.md for the presto-connector module - Fix spotbugs-maven-plugin version (4.9.8 → 4.9.8.3) - Add pluginManagement and missing default plugin versions to pom.xml
20001020ycx
force-pushed
the
feat/2026-05-21-presto-connector-taskfile
branch
from
148ce52 to
07753eb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@presto-connector/lint-tasks.yaml`:
- Around line 20-24: Two lint tasks, lint-check and lint, both run the identical
command mvn compile spotbugs:check which is misleading because SpotBugs does not
auto-fix; either remove one duplicate task (keep a single canonical task name,
e.g., lint) and update references to use that, or keep both but add a clear
comment above each task explaining that SpotBugs only performs static analysis
and cannot auto-fix (so task names like lint:fix are not applicable); update the
task definitions for lint-check and lint accordingly to avoid confusion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3ae657a7-a9c6-4d98-8e55-23faac424364
📒 Files selected for processing (6)
presto-connector/.eclipse-formatter.xmlpresto-connector/README.mdpresto-connector/lint-tasks.yamlpresto-connector/pom.xmlpresto-connector/spotbugs-exclude.xmlpresto-connector/taskfile.yaml
kirkrodrigues
left a comment
kirkrodrigues
left a comment
There was a problem hiding this comment.
It seems we don't have yaml linting in this PR. In order to ensure consistent style with our other repos, perhaps you could submit a PR to add yaml linting first and then we could build this PR on top of that?
I'm still in the process of reviewing the rest of this PR.
| <shade.plugin.version>3.6.2</shade.plugin.version> | ||
| <site.plugin.version>3.21.0</site.plugin.version> | ||
| <spotbugs.plugin.version>4.9.8.3</spotbugs.plugin.version> | ||
| <spotless.plugin.version>3.5.1</spotless.plugin.version> |
There was a problem hiding this comment.
Any reason to not use the latest instead?
There was a problem hiding this comment.
Actually i did a pass on the version number before posting the PR, this is where I look for the latest version number, is this the correct place to look it up: https://mvnrepository.com/artifact?
From mvn repo, spotless 3.5.1 is the latest according to here. and spotbugs 4.9.8.3 is the latest acoording to here
| <resources.plugin.version>3.5.0</resources.plugin.version> | ||
| <shade.plugin.version>3.6.2</shade.plugin.version> | ||
| <site.plugin.version>3.21.0</site.plugin.version> | ||
| <spotbugs.plugin.version>4.9.8.3</spotbugs.plugin.version> |
There was a problem hiding this comment.
| <spotbugs.plugin.version>4.9.8.3</spotbugs.plugin.version> | |
| <spotbugs.plugin.version>4.9.8</spotbugs.plugin.version> |
Where'd you find the `.3?
There was a problem hiding this comment.
might be a typo when updating the version
There was a problem hiding this comment.
Actually no, here is where I found, did i look up in the right place: https://mvnrepository.com/artifact/com.github.spotbugs/spotbugs-maven-plugin
This is how we use it right? So its groupId and artifactId matches to the link above
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>${spotbugs.plugin.version}</version>
</plugin>
Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
presto-connector/pom.xml (1)
288-288:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIncorrect package name in import order configuration.
The import order specifies
com.facebook.presto.plugin.clp, but this project's groupId iscom.yscope.presto(Line 7). This appears to be a copy-paste error from another project.📦 Proposed fix to use the correct package
<importOrder> - <order>java|javax,,com.facebook.presto.plugin.clp</order> + <order>java|javax,,com.yscope.presto</order> </importOrder>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@presto-connector/pom.xml` at line 288, The import-order configuration contains a wrong package name: replace the incorrect package token "com.facebook.presto.plugin.clp" in the <order> element with the correct project package "com.yscope.presto.plugin.clp" so the ordering entry reads "java|javax,,com.yscope.presto.plugin.clp"; update the <order> value and verify there are no other stale references to com.facebook.presto.plugin.clp (search for that symbol) to ensure import ordering matches the project's groupId.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@presto-connector/pom.xml`:
- Line 288: The import-order configuration contains a wrong package name:
replace the incorrect package token "com.facebook.presto.plugin.clp" in the
<order> element with the correct project package "com.yscope.presto.plugin.clp"
so the ordering entry reads "java|javax,,com.yscope.presto.plugin.clp"; update
the <order> value and verify there are no other stale references to
com.facebook.presto.plugin.clp (search for that symbol) to ensure import
ordering matches the project's groupId.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 30db663e-85d9-49b3-b551-96e7ca097cba
📒 Files selected for processing (1)
presto-connector/pom.xml
Done, please review, #10 |
… into feat/2026-05-21-presto-connector-taskfile
2 participants
Description
This PR adds Taskfiles and related configuration files for building and linting the Java plugin - the coordinator-side of the Presto CLP connector. See the updated README for usage instructions.
Reviewer's note
Given that this change involves substantial changes, I have split them across two commits for easier review:
Commit 1 (no content review needed) —
Spotlessauto-formatting of all source code.Commit 2 (please review carefully) — New documentation and Taskfile infrastructure for building and linting the Java plugin, largely following the example from
log4j2-appenders. Each file requiring review is listed below with some design notes to help review:pom.xml: Addsspotlessandspotbugswith the latest version.taskfile.yaml/lint-tasks.yaml: Simple task definition forbuild,test,lint:check, andlint:fix.spotbugs-exclude.xml: Two exclusions with justifications are confirmed false positives. The remaining three (guarded by TODO without any justification) are valid SpotBugs findings that will be fixed in a future PR, tracked in Fix SpotBugs findings currently suppressed in spotbugs-exclude.xml #8 ..eclipse-formatter.xml: Copied fromlog4j2-appenderswith indentation padding changed from 4 spaces to 2 spaces.Breaking changes
None
Validation performed
task build,task lint:checkandtask lint:fix. No error returned.task testfrom Java pluginSELECT timestamp, CLP_GET_JSON_STRING() from clp.default.default limit 100Checklist
(fixes #N)or(resolves #N)syntax in the title.Summary by CodeRabbit
Release Notes
Documentation
Chores