Skip to content
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

Handle IS NULL/IS NOT NULL filtering when ExpressionTransformer outputs null #14433

Open
itschrispeck opened this issue Nov 12, 2024 · 3 comments

Comments

@itschrispeck
Copy link
Collaborator

itschrispeck commented Nov 12, 2024

Take the transformation:

        {
          "columnName": "col1",
          "transformFunction": "CASE WHEN col2 IS NOT NULL THEN col2 ELSE null END"
        },

Although the function returns null, typical null value handling does not apply. Ideally, if an ExpressionTransformer returns null, the value can still be filtered via IS NULL/IS NOT NULL. To maintain backwards compatibility we could add a new config per transformFunction, enableNullHandling:

        {
          "columnName": "col1",
          "transformFunction": "CASE WHEN col2 IS NOT NULL THEN col2 ELSE null END",
          "enableNullHandling": true
        },
@Jackie-Jiang
Copy link
Contributor

cc @swaminathanmanish

What is the current behavior? What is the value produced when the expression returns null?
We should just use the setting in table config instead of introducing a new flag.

@itschrispeck
Copy link
Collaborator Author

itschrispeck commented Nov 14, 2024

Current behavior is that the value is an empty string e.g. select col1 from tbl where col1 is not null gives:

  "resultTable": {
    "dataSchema": {
      "columnNames": [
        "col1"
      ],
      "columnDataTypes": [
        "STRING"
      ]
    },
    "rows": [
      [
        ""
      ],
      ...
  }

Conversely, is null does not match any docs.

I'd also prefer avoiding a new config, I wasn't sure if this should be considered a bug since changing the behavior would be backwards incompatible. Though looking at this briefly, it seems NullValueTransformer is intended to handle null output from ExpressionTransformer

@Jackie-Jiang
Copy link
Contributor

Correct. I feel the real issue is that null is not properly set in the ExpressionTransformer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants