mattcasters commented on code in PR #8778:
URL: https://github.com/apache/hop/pull/8778#discussion_r4229677938
##########
plugins/actions/truncatetables/src/main/java/org/apache/hop/workflow/actions/truncatetables/ActionTruncateTablesDialog.java:
##########
@@ -302,26 +307,64 @@ private void ok() {
}
private void getTableName() {
- DatabaseMeta databaseMeta =
getWorkflowMeta().findDatabase(wConnection.getText(), variables);
+ String connectionName = wConnection.getText();
+ if (Utils.isEmpty(connectionName)) {
+ MessageBox mb = new MessageBox(shell, SWT.OK | SWT.ICON_ERROR);
+ mb.setText(BaseMessages.getString(PKG, "System.Dialog.Error.Title"));
+ mb.setMessage(BaseMessages.getString(PKG,
"ActionTruncateTables.NoDbConnection"));
+ mb.open();
+ return;
+ }
+
+ DatabaseMeta databaseMeta = getWorkflowMeta().findDatabase(connectionName,
variables);
if (databaseMeta != null) {
try (Database database = new Database(loggingObject, variables,
databaseMeta)) {
database.connect();
- String[] tableNames = database.getTablenames();
- Arrays.sort(tableNames);
+ Map<String, Collection<String>> tableMap = database.getTableMap();
+ List<String> displayList = new ArrayList<>();
+ List<String[]> schemaTableList = new ArrayList<>();
+
+ if (tableMap != null && !tableMap.isEmpty()) {
+ List<String> schemas = new ArrayList<>(tableMap.keySet());
+ Collections.sort(schemas, String.CASE_INSENSITIVE_ORDER);
+ for (String schema : schemas) {
+ List<String> tables = new ArrayList<>(tableMap.get(schema));
+ Collections.sort(tables, String.CASE_INSENSITIVE_ORDER);
+ for (String table : tables) {
+ String displayName = Utils.isEmpty(schema) ? table : schema +
"." + table;
+ displayList.add(displayName);
+ schemaTableList.add(new String[] {table, Const.NVL(schema, "")});
+ }
+ }
+ } else {
+ // Fallback to simple table names if tableMap is empty
+ String[] tableNames = database.getTablenames();
Review Comment:
**[nit]** This fallback cannot fill the list. `getTablenames()` is built
from `getTableMap()`, so an empty map produces an empty array. `getTableMap()`
also swallows `SQLException` and returns that empty map, which means a metadata
failure opens an empty picker instead of the connection error dialog.
**Suggestion:** Remove the fallback. If table metadata cannot be read,
surface that error rather than opening an empty selection dialog.
##########
plugins/actions/truncatetables/src/main/java/org/apache/hop/workflow/actions/truncatetables/ActionTruncateTablesDialog.java:
##########
@@ -302,26 +307,64 @@ private void ok() {
}
private void getTableName() {
- DatabaseMeta databaseMeta =
getWorkflowMeta().findDatabase(wConnection.getText(), variables);
+ String connectionName = wConnection.getText();
+ if (Utils.isEmpty(connectionName)) {
+ MessageBox mb = new MessageBox(shell, SWT.OK | SWT.ICON_ERROR);
+ mb.setText(BaseMessages.getString(PKG, "System.Dialog.Error.Title"));
+ mb.setMessage(BaseMessages.getString(PKG,
"ActionTruncateTables.NoDbConnection"));
+ mb.open();
+ return;
+ }
+
+ DatabaseMeta databaseMeta = getWorkflowMeta().findDatabase(connectionName,
variables);
if (databaseMeta != null) {
try (Database database = new Database(loggingObject, variables,
databaseMeta)) {
database.connect();
- String[] tableNames = database.getTablenames();
- Arrays.sort(tableNames);
+ Map<String, Collection<String>> tableMap = database.getTableMap();
+ List<String> displayList = new ArrayList<>();
+ List<String[]> schemaTableList = new ArrayList<>();
+
+ if (tableMap != null && !tableMap.isEmpty()) {
+ List<String> schemas = new ArrayList<>(tableMap.keySet());
+ Collections.sort(schemas, String.CASE_INSENSITIVE_ORDER);
Review Comment:
**[bug]** `String.CASE_INSENSITIVE_ORDER` throws `NullPointerException` when
a schema key is null. `Database.getTableMap()` stores a null key when JDBC
returns null for both `TABLE_SCHEM` and `TABLE_CAT`: those columns start as
`""`, `ResultSet.getString` overwrites them with null, and the empty check then
assigns that null catalog. SQLite and other schema-less drivers do this. The
previous `getTablenames()` path never sorted those keys, so this button listed
the tables and left schema blank. The NPE is not a `HopDatabaseException`, so
the new catch does not open the error dialog and the click fails instead.
**Suggestion:** Sort with
`Comparator.nullsFirst(String.CASE_INSENSITIVE_ORDER)` (same for table names).
A null schema should stay "no schema"; `Utils.isEmpty` and `Const.NVL` already
do that once the sort survives. Catch `Exception` here so an unexpected failure
still shows the error dialog.
##########
plugins/actions/truncatetables/src/main/java/org/apache/hop/workflow/actions/truncatetables/ActionTruncateTables.java:
##########
@@ -266,4 +270,28 @@ public boolean isArgFromPrevious() {
public void setArgFromPrevious(boolean argFromPrevious) {
this.argFromPrevious = argFromPrevious;
}
+
+ @Override
+ public void check(
+ List<ICheckResult> remarks,
+ WorkflowMeta workflowMeta,
+ IVariables variables,
+ IHopMetadataProvider metadataProvider) {
+
+ if (!argFromPrevious) {
Review Comment:
**[suggestion]** `check()` warns when no tables are selected, but not when
the connection is missing. That is the failure this change fixes at runtime: an
action with a table name and an empty connection still passes Verify Workflow
and only fails when it runs. `workflowMeta` is unused, so a connection name
that is set but no longer in metadata is also ignored.
**Suggestion:** When `Utils.isEmpty(variables.resolve(connection))`, or
`workflowMeta.findDatabase(...)` returns null, add a warning using
`ActionTruncateTables.NoDbConnection`. Do this even when `argFromPrevious` is
set, because previous rows still need a connection.
##########
plugins/actions/truncatetables/src/main/java/org/apache/hop/workflow/actions/truncatetables/ActionTruncateTablesDialog.java:
##########
@@ -302,26 +307,64 @@ private void ok() {
}
private void getTableName() {
- DatabaseMeta databaseMeta =
getWorkflowMeta().findDatabase(wConnection.getText(), variables);
+ String connectionName = wConnection.getText();
+ if (Utils.isEmpty(connectionName)) {
+ MessageBox mb = new MessageBox(shell, SWT.OK | SWT.ICON_ERROR);
+ mb.setText(BaseMessages.getString(PKG, "System.Dialog.Error.Title"));
+ mb.setMessage(BaseMessages.getString(PKG,
"ActionTruncateTables.NoDbConnection"));
+ mb.open();
+ return;
+ }
+
+ DatabaseMeta databaseMeta = getWorkflowMeta().findDatabase(connectionName,
variables);
if (databaseMeta != null) {
Review Comment:
**[suggestion]** The new guard only covers an empty connection name. If the
name is non-empty but `findDatabase` returns null (deleted or unresolved
metadata), this method returns with no message and the button looks dead.
**Suggestion:** In the null `databaseMeta` case, show the same error dialog
as the empty-connection check (`ActionTruncateTables.NoDbConnection`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]