-
Notifications
You must be signed in to change notification settings - Fork 299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add port number as a DBS_PORT parameter in Teradata JDBC Connection String #2452
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,12 +21,31 @@ | |
|
||
import com.amazonaws.athena.connectors.jdbc.JdbcEnvironmentProperties; | ||
|
||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
import static com.amazonaws.athena.connector.lambda.connection.EnvironmentConstants.DATABASE; | ||
import static com.amazonaws.athena.connector.lambda.connection.EnvironmentConstants.DEFAULT; | ||
import static com.amazonaws.athena.connector.lambda.connection.EnvironmentConstants.HOST; | ||
import static com.amazonaws.athena.connector.lambda.connection.EnvironmentConstants.PORT; | ||
|
||
public class TeradataEnvironmentProperties extends JdbcEnvironmentProperties | ||
{ | ||
@Override | ||
public Map<String, String> connectionPropertiesToEnvironment(Map<String, String> connectionProperties) | ||
{ | ||
HashMap<String, String> environment = new HashMap<>(); | ||
Check warning on line 37 in athena-teradata/src/main/java/com/amazonaws/athena/connectors/teradata/TeradataEnvironmentProperties.java Codecov / codecov/patchathena-teradata/src/main/java/com/amazonaws/athena/connectors/teradata/TeradataEnvironmentProperties.java#L37
|
||
// Default port for teradata is 1025 | ||
String port = connectionProperties.getOrDefault(PORT, "1025"); | ||
Check warning on line 39 in athena-teradata/src/main/java/com/amazonaws/athena/connectors/teradata/TeradataEnvironmentProperties.java Codecov / codecov/patchathena-teradata/src/main/java/com/amazonaws/athena/connectors/teradata/TeradataEnvironmentProperties.java#L39
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please move this to the Teradata constants; not here. |
||
|
||
// Construct the JDBC connection string and include the port as a DBS_PORT parameter | ||
String connectionString = getConnectionStringPrefix(connectionProperties) + connectionProperties.get(HOST) | ||
+ getDatabase(connectionProperties) + ",DBS_PORT=" + port + getJdbcParameters(connectionProperties); | ||
Check warning on line 43 in athena-teradata/src/main/java/com/amazonaws/athena/connectors/teradata/TeradataEnvironmentProperties.java Codecov / codecov/patchathena-teradata/src/main/java/com/amazonaws/athena/connectors/teradata/TeradataEnvironmentProperties.java#L42-L43
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is assuming a new change; will this break existing customers? what exactly promoted this change? I want to make sure this is backward compatible; i.e.; existing customers who are upgrading won't have their environments broken. |
||
|
||
environment.put(DEFAULT, connectionString); | ||
return environment; | ||
Check warning on line 46 in athena-teradata/src/main/java/com/amazonaws/athena/connectors/teradata/TeradataEnvironmentProperties.java Codecov / codecov/patchathena-teradata/src/main/java/com/amazonaws/athena/connectors/teradata/TeradataEnvironmentProperties.java#L45-L46
|
||
} | ||
|
||
@Override | ||
protected String getConnectionStringPrefix(Map<String, String> connectionProperties) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add a unit test once questions below are answered.