Add Teradata connector

[electrum]

Release notes

(x) Release notes are required, with the following suggested text:

## Connectors
* Add Teradata connector. ({issue}`xxx`)

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• The CI workflow change replaces the entire impacted-modules matrix with only the Teradata entry—please double-check that other connectors and core modules are still tested as expected in your impacted build matrix.
• TestTeradataConnectorTest overrides and aborts dozens of inherited tests to skip unsupported operations; consider using a reusable JUnit conditional annotation or a custom test filter rather than duplicating override methods.
• The ClearScape HTTP client and exception handling logic has a lot of repetition—extract a shared response handler or utility to centralize status-code checks and JSON deserialization.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The CI workflow change replaces the entire impacted-modules matrix with only the Teradata entry—please double-check that other connectors and core modules are still tested as expected in your impacted build matrix.
- TestTeradataConnectorTest overrides and aborts dozens of inherited tests to skip unsupported operations; consider using a reusable JUnit conditional annotation or a custom test filter rather than duplicating override methods.
- The ClearScape HTTP client and exception handling logic has a lot of repetition—extract a shared response handler or utility to centralize status-code checks and JSON deserialization.

## Individual Comments

### Comment 1
<location> `plugin/trino-teradata/src/main/java/io/trino/plugin/teradata/TeradataClient.java:141` </location>
<code_context>
+                isCaseSensitive ? TERADATA_STRING_PUSHDOWN : CASE_INSENSITIVE_CHARACTER_PUSHDOWN);
+    }
+
+    private boolean deriveCaseSensitivity(CaseSensitivity caseSensitivity)
+    {
+        return switch (teradataJDBCCaseSensitivity) {
</code_context>

<issue_to_address>
**issue (bug_risk):** The deriveCaseSensitivity method's default branch may not behave as intended.

The default branch should return whether 'caseSensitivity' equals 'CASE_SENSITIVE', or explicitly handle the 'AS_DEFINED' case to ensure correct behavior.
</issue_to_address>

### Comment 2
<location> `plugin/trino-teradata/src/main/java/io/trino/plugin/teradata/TeradataClient.java:347` </location>
<code_context>
+    {
+        int precision = typeHandle.requiredColumnSize();
+        int scale = typeHandle.requiredDecimalDigits();
+        if (precision > Decimals.MAX_PRECISION) {
+            // this will trigger for number(*) as precision is 40
+            return Optional.of(decimalColumnMapping(createDecimalType(Decimals.MAX_PRECISION, scale)));
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Handling of precision greater than Decimals.MAX_PRECISION may lead to silent truncation.

Recommend logging a warning or raising an exception when precision exceeds MAX_PRECISION to alert users to possible truncation or rounding.

Suggested implementation:

```java
    private Optional<ColumnMapping> numberMapping(JdbcTypeHandle typeHandle)
    {
        int precision = typeHandle.requiredColumnSize();
        int scale = typeHandle.requiredDecimalDigits();
        if (precision > Decimals.MAX_PRECISION) {
            // this will trigger for number(*) as precision is 40
            log.warn("Precision {} exceeds maximum supported precision {}. Truncating to {}. This may lead to loss of precision or rounding.",
                    precision, Decimals.MAX_PRECISION, Decimals.MAX_PRECISION);
            return Optional.of(decimalColumnMapping(createDecimalType(Decimals.MAX_PRECISION, scale)));
        }
        return Optional.of(decimalColumnMapping(createDecimalType(precision, scale)));
    }

```

```java
    private static final org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(TeradataClient.class);

```

If the class already has a logger defined, you do not need to add the `private static final org.slf4j.Logger log ...` line. If the logger is named differently, use the existing logger variable.
</issue_to_address>

### Comment 3
<location> `plugin/trino-teradata/src/main/java/io/trino/plugin/teradata/TeradataClientModule.java:49` </location>
<code_context>
+    {
+        Driver driver = DriverManager.getDriver(config.getConnectionUrl());
+        Properties connectionProperties = new Properties();
+        connectionProperties.setProperty("LOGMECH", "TD2");
+        return DriverConnectionFactory.builder(driver, config.getConnectionUrl(), credentialProvider)
+                .setConnectionProperties(connectionProperties)
</code_context>

<issue_to_address>
**suggestion:** Hardcoding LOGMECH to TD2 may limit authentication flexibility.

Consider allowing LOGMECH to be set through TeradataConfig or connection properties to support different authentication methods.

Suggested implementation:

```java
        Properties connectionProperties = new Properties();
        String logMech = teradataConfig.getLogMech();
        if (logMech == null || logMech.isEmpty()) {
            logMech = "TD2";
        }
        connectionProperties.setProperty("LOGMECH", logMech);
        return DriverConnectionFactory.builder(driver, config.getConnectionUrl(), credentialProvider)
                .setConnectionProperties(connectionProperties)
                .setOpenTelemetry(openTelemetry).build();

```

You will also need to:
1. Add a `logMech` property to the `TeradataConfig` class, with a getter method (`getLogMech()`).
2. Ensure this property can be set via configuration (e.g., config file or environment variable).
</issue_to_address>

### Comment 4
<location> `plugin/trino-teradata/src/test/java/io/trino/plugin/teradata/integration/TestTeradataConnectorTest.java:241-246` </location>
<code_context>
+        assertUpdate("DROP TABLE " + tableName);
+    }
+
+    @Override // Overriding this test case as Teradata does not support negative dates.
+    @Test
+    public void testDateYearOfEraPredicate()
+    {
+        assertQuery("SELECT orderdate FROM orders WHERE orderdate = DATE '1997-09-14'", "VALUES DATE '1997-09-14'");
</code_context>

<issue_to_address>
**suggestion (testing):** Negative date support is skipped.

Consider adding a test to verify that attempting to use negative dates results in a clear and expected error, ensuring the connector handles this case gracefully.

```suggestion
    @Override // Overriding this test case as Teradata does not support negative dates.
    @Test
    public void testDateYearOfEraPredicate()
    {
        assertQuery("SELECT orderdate FROM orders WHERE orderdate = DATE '1997-09-14'", "VALUES DATE '1997-09-14'");
    }

    @Test
    public void testNegativeDatePredicateFailsGracefully()
    {
        // Teradata does not support negative dates (e.g., year 0000 or negative years)
        assertQueryFails(
                "SELECT DATE '-0001-01-01'",
                ".*Invalid date.*|.*Teradata does not support negative dates.*"
        );
    }
```
</issue_to_address>

### Comment 5
<location> `plugin/trino-teradata/src/test/java/io/trino/plugin/teradata/integration/TestTeradataConnectorTest.java:263-267` </location>
<code_context>
+        }
+    }
+
+    @Override // Overriding this test case as Teradata doesn't have support to (k, v) AS VALUES in insert statement
+    @Test
+    public void testCharVarcharComparison()
+    {
+        String testTableName = "test_char_varchar";
</code_context>

<issue_to_address>
**suggestion (testing):** Test for char/varchar comparison is customized for Teradata.

Please add test cases for empty strings, trailing spaces, and NULL values to cover more edge scenarios in char/varchar comparisons.

```suggestion
        try (TestTable table = newTrinoTable(
                testTableName,
                "(k int, v char(3))",
                List.of(
                        "-1, CAST(NULL AS char(3))",
                        "3, CAST('   ' AS char(3))",
                        "6, CAST('x  ' AS char(3))",
                        "7, CAST('' AS char(3))",         // empty string
                        "8, CAST('a  ' AS char(3))",      // trailing spaces
                        "9, CAST('abc' AS char(3))",      // no trailing spaces
                        "10, CAST(' ' AS char(3))"        // single space
                )
        )) {
            // Existing assertions
            assertQuery("SELECT k, v FROM " + table.getName() + " WHERE v = CAST('  ' AS varchar(2))", "VALUES (3, '   ')");
            assertQuery("SELECT k, v FROM " + table.getName() + " WHERE v = CAST('  ' AS varchar(4))", "VALUES (3, '   ')");
            assertQuery("SELECT k, v FROM " + table.getName() + " WHERE v = CAST('x ' AS varchar(2))", "VALUES (6, 'x  ')");

            // Edge case: empty string
            assertQuery("SELECT k, v FROM " + table.getName() + " WHERE v = CAST('' AS varchar(3))", "VALUES (7, '   ')");

            // Edge case: trailing spaces
            assertQuery("SELECT k, v FROM " + table.getName() + " WHERE v = CAST('a' AS varchar(1))", "VALUES (8, 'a  ')");
            assertQuery("SELECT k, v FROM " + table.getName() + " WHERE v = CAST('a  ' AS varchar(3))", "VALUES (8, 'a  ')");

            // Edge case: no trailing spaces
            assertQuery("SELECT k, v FROM " + table.getName() + " WHERE v = CAST('abc' AS varchar(3))", "VALUES (9, 'abc')");

            // Edge case: single space
            assertQuery("SELECT k, v FROM " + table.getName() + " WHERE v = CAST(' ' AS varchar(1))", "VALUES (10, '   ')");

            // Edge case: NULL value
            assertQuery("SELECT k, v FROM " + table.getName() + " WHERE v IS NULL", "VALUES (-1, CAST(NULL AS char(3)))");
        }
```
</issue_to_address>

### Comment 6
<location> `docs/src/main/sphinx/connector/teradata.md:78-79` </location>
<code_context>
+
+Because Trino and Teradata each support types that the other does not, this
+connector {ref}`modifies some types <type-mapping-overview>` when reading data.
+Refer to the following sections for type mapping in when reading data from
+Teradata to Trino.
+
+### Teradata type to Trino type mapping
</code_context>

<issue_to_address>
**issue (typo):** Remove the extra word 'in' from the sentence for clarity.

It should say: 'Refer to the following sections for type mapping when reading data from Teradata to Trino.'

```suggestion
Refer to the following sections for type mapping when reading data from
Teradata to Trino.
```
</issue_to_address>

### Comment 7
<location> `docs/src/main/sphinx/connector/teradata.md:174-175` </location>
<code_context>
+
+## SQL support
+
+The connector provides read access access to data and metadata in
+a Teradata database.  The connector supports the {ref}`globally available
+<sql-globally-available>` and {ref}`read operation <sql-read-operations>`
+statements.
</code_context>

<issue_to_address>
**issue (typo):** Remove the repeated word 'access' in 'read access access to data'.

It should be: 'The connector provides read access to data and metadata in a Teradata database.'
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (testing): Negative date support is skipped.

Consider adding a test to verify that attempting to use negative dates results in a clear and expected error, ensuring the connector handles this case gracefully.

Suggested change

	@Override // Overriding this test case as Teradata does not support negative dates.
	@Test
	public void testDateYearOfEraPredicate()
	{
	assertQuery("SELECT orderdate FROM orders WHERE orderdate = DATE '1997-09-14'", "VALUES DATE '1997-09-14'");
	}
	@Override // Overriding this test case as Teradata does not support negative dates.
	@Test
	public void testDateYearOfEraPredicate()
	{
	assertQuery("SELECT orderdate FROM orders WHERE orderdate = DATE '1997-09-14'", "VALUES DATE '1997-09-14'");
	}
	@Test
	public void testNegativeDatePredicateFailsGracefully()
	{
	// Teradata does not support negative dates (e.g., year 0000 or negative years)
	assertQueryFails(
	"SELECT DATE '-0001-01-01'",
	".*Invalid date.*|.*Teradata does not support negative dates.*"
	);
	}

[Copilot]

Pull Request Overview

This PR introduces a new Teradata connector for Trino, enabling integration with Teradata databases. The implementation includes core connector functionality, HTTP client for ClearScape API integration, comprehensive test coverage, and documentation.

• Adds Teradata JDBC connector with type mapping and query capabilities
• Implements ClearScape Analytics cloud environment management for testing
• Includes integration tests with Maven profile configuration
• Provides complete documentation for setup and usage

Reviewed Changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pom.xml	Adds trino-teradata module to parent POM
plugin/trino-teradata/pom.xml	Maven configuration with dependencies and test profiles
plugin/trino-teradata/src/main/java/io/trino/plugin/teradata/*	Core connector implementation including plugin, client, and configuration
plugin/trino-teradata/src/test/java/io/trino/plugin/teradata/integration/*	Integration test infrastructure and test cases
plugin/trino-teradata/src/test/java/io/trino/plugin/teradata/integration/clearscape/*	ClearScape API client and environment management
docs/src/main/sphinx/connector/teradata.md	Connector documentation
.github/workflows/ci.yml	CI workflow configuration for Teradata tests

Comments suppressed due to low confidence (1)

plugin/trino-teradata/src/test/java/io/trino/plugin/teradata/integration/clearscape/ClearScapeManager.java:1

• This SQL query uses string formatting without proper quoting, which could lead to SQL injection if schema or table names contain special characters. Use the 'quoted()' method or parameterized queries to safely construct the SQL.

/*

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The default case returns 'caseSensitivity != null' which evaluates to a boolean. However, if caseSensitivity is null, this returns false. This logic is unclear and may not handle the AS_DEFINED case correctly. Consider explicitly handling the AS_DEFINED case: 'case AS_DEFINED -> caseSensitivity != null && caseSensitivity == CASE_SENSITIVE;' or clarify the intended behavior.

Suggested change

	default -> caseSensitivity != null;
	case AS_DEFINED -> caseSensitivity != null && caseSensitivity == CASE_SENSITIVE;

[sc250072]

We’ve implemented additional logon mechanisms such as SECRET, BEARER, and JWT, and plan to raise a separate PR for them later. For now, we can proceed with the current setup. In the upcoming PR, we’ll reintroduce the LogonMechanism config along with the new mechanisms.

[sc250072]

How can we ensure that only Teradata-specific tests run in the PR? In another PR we raised, tests from other plugins were also executed. Is there a way to control this behavior through the CI workflow?

[sc250072]

How can we ensure that only Teradata-specific tests run in the PR? In another PR we raised, tests from other plugins were also executed. Is there a way to control this behavior through the CI workflow?

396f279 - got it from here. Please ignore this message