You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hop.apache.org by "hansva (via GitHub)" <gi...@apache.org> on 2026/09/24 14:54:53 UTC
[PR] DatabaseMeta was not correctly loaded in Dynamic SQL, fixes #4471 (hop)
hansva opened a new pull request, #8584:
URL: https://github.com/apache/hop/pull/8584
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Run `mvn clean install apache-rat:check` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [ ] If you have a group of commits related to the same change, please squash your commits into one and force push your branch using `git rebase -i`.
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable.
To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] DatabaseMeta was not correctly loaded in Dynamic SQL, fixes #4471 (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on code in PR #8584:
URL: https://github.com/apache/hop/pull/8584#discussion_r4106369645
##########
plugins/transforms/dynamicsqlrow/src/main/java/org/apache/hop/pipeline/transforms/dynamicsqlrow/DynamicSqlRowMeta.java:
##########
@@ -333,6 +344,34 @@ public void analyseImpact(
}
}
+ /**
+ * Looks up the connection by name every time it's needed. The resolved connection isn't kept on
+ * the meta: a freshly loaded pipeline would otherwise report no template fields until the dialog
+ * was opened.
+ */
+ private DatabaseMeta loadDatabaseMeta(IVariables variables, IHopMetadataProvider metadataProvider)
+ throws HopTransformException {
+ if (Utils.isEmpty(connection)) {
+ return null;
+ }
+ String realConnection = variables.resolve(connection);
+ DatabaseMeta databaseMeta;
+ try {
+ databaseMeta = metadataProvider.getSerializer(DatabaseMeta.class).load(realConnection);
+ } catch (HopException e) {
+ throw new HopTransformException(
+ BaseMessages.getString(
+ PKG, "DynamicSQLRowMeta.Exception.ConnectionNotFound", realConnection),
+ e);
+ }
+ if (databaseMeta == null) {
Review Comment:
**[suggestion]** A missing connection is now a hard `HopTransformException` from every `getFields` caller. That matches `TableInputMeta` for a concrete unknown name, and `getFieldsWithUnknownConnectionThrows` locks it in, but an unset variable is left as `${NAME}` by `StringUtil.substitute` and then hits this same throw. `DynamicSqlRowDialog.getSqlReservedWords()` already returns no keywords when the resolved name still starts with `${`. Field discovery and pipeline verify call `getFields` first, so a connection variable that is only set at runtime now fails the whole field chain instead of omitting the template fields.
**Suggestion:** Return null when the resolved name is empty or still contains `${`, and keep the throw for a resolved name that is not in the metadata store. `check()` can keep reporting the error.
##########
plugins/transforms/dynamicsqlrow/src/main/java/org/apache/hop/pipeline/transforms/dynamicsqlrow/DynamicSqlRowMeta.java:
##########
@@ -303,6 +313,7 @@ public void analyseImpact(
IHopMetadataProvider metadataProvider)
throws HopTransformException {
+ DatabaseMeta databaseMeta = loadDatabaseMeta(variables, metadataProvider);
Review Comment:
**[suggestion]** `loadDatabaseMeta` returns null when `connection` is empty, but `analyseImpact` still calls `databaseMeta.getDatabaseName()` for every field in `out`. `out` is `prev.clone()`, so any upstream fields turn this into an NPE and `PipelineMeta.analyseImpact` fails for the whole pipeline. `getFields` and `check` both handle that null. The crash was already possible when the old field was unset, but this is the only new caller that ignores the null return.
**Suggestion:** If `databaseMeta` is null, return before `getFields` and the impact loop. An empty connection has nothing to report.
##########
plugins/transforms/dynamicsqlrow/src/main/java/org/apache/hop/pipeline/transforms/dynamicsqlrow/DynamicSqlRowMeta.java:
##########
@@ -141,6 +138,7 @@ public void getFields(
IHopMetadataProvider metadataProvider)
throws HopTransformException {
+ DatabaseMeta databaseMeta = loadDatabaseMeta(variables, metadataProvider);
if (databaseMeta == null) {
Review Comment:
**[bug]** `getFields()` returns only when the connection is missing. With a connection name set and the template SQL still empty (`setDefault()` stores `""`), execution falls through to `Database.getQueryFields`. The unconnected call returns null, then `db.connect()` runs and the second call prepares an empty statement. That becomes `HopTransformException` (`DynamicSQLRowMeta.Exception.ErrorObtainingFields`). `PipelineMeta.getTransformFields` does not catch it, and `checkTransforms` treats the failure as fatal (`stopChecking = true`), so Show Output Fields, Get Fields, and Verify fail for every downstream transform. It is easy to hit: in a project with one connection the dialog auto-selects it and OK stores the name without requiring SQL. `check()` already skips an empty template (line 269), and `processRow()` rejects empty SQL only at execution time. The new tests never pass an empty SQL with a connection, and they stub `getQueryFields` so this path stays green.
**Suggestion:** Resolve the template first and return before connecting when it is empty, including a variable that resolves to empty: `if (databaseMeta == null || Utils.isEmpty(realSql)) return;`. Add a test that a set connection plus empty SQL leaves the input row unchanged and does not construct a `Database`.
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] DatabaseMeta was not correctly loaded in Dynamic SQL, fixes #4471 (hop)
Posted by "hansva (via GitHub)" <gi...@apache.org>.
hansva merged PR #8584:
URL: https://github.com/apache/hop/pull/8584
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org