feat: support special characters in column names for filter parsing #557

GergesHany · 2025-10-02T14:16:59Z

Description

Enhanced filter regex to handle quoted column names with double quotes and backticks
Added support for column names containing hyphens, spaces, dots, and other special characters
Updated GetFilter() method to extract column names from multiple capture groups
Added comprehensive test suite with 15+ test cases covering various scenarios:
- Quoted column names with spaces and special characters
- Mixed quoting styles (double quotes and backticks)
- Complex filters with AND/OR logical operators
- Edge cases and error conditions
Resolves issue with parsing filters for columns like "user-id", "order date", "item.price"

Examples now supported:

"user-id" > 5 and order date = "2024-01-01"
item.price >= 100.50 or "column name" != "value"

Fixes #498

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Note

Improves filter parsing to support quoted column names, broader value formats, logical operators, and adds a comprehensive test suite.

Filter parsing (types/stream_configured.go):
- Trim input filter via strings.TrimSpace.
- Replace regex to support double-quoted column names (incl. spaces, hyphens, dots) and unquoted identifiers; accept quoted strings, negative/decimal numbers, scientific notation, NULL values.
- Preserve case of logical operators while matching case-insensitively (and/or).
- Extract column via helper to combine capture groups; adjust indices and LogicalOperator handling.
Tests (types/stream_configured_test.go):
- Add extensive test suite covering valid/invalid cases: quoted identifiers/values, mixed spacing, operator variants, numeric formats, logical operators, edge/error conditions.

^{Written by Cursor Bugbot for commit a61b0fc. This will update automatically on new commits. Configure here.}

- Enhanced filter regex to handle quoted column names with double quotes and backticks - Added support for column names containing hyphens, spaces, dots, and other special characters - Updated GetFilter() method to extract column names from multiple capture groups - Added comprehensive test suite with 15+ test cases covering various scenarios: - Quoted column names with spaces and special characters - Mixed quoting styles (double quotes and backticks) - Complex filters with AND/OR logical operators - Edge cases and error conditions - Resolves issue with parsing filters for columns like "user-id", "order date", "item.price" Examples now supported: - "user-id" > 5 and `order date` = "2024-01-01" - `item.price` >= 100.50 or "column name" != "value"

CLAassistant · 2025-10-02T14:17:07Z

All committers have signed the CLA.

vaibhav-datazip · 2025-10-03T14:09:33Z

Hi @GergesHany ,
The PR needs to be raised on top of the staging branch not the master, can you please fix this.

GergesHany · 2025-10-03T14:15:13Z

@vaibhav-datazip Done

…ame-in-filter

types/stream_configured.go

… and use regex match indices directly in Condition struct initialization.

GergesHany · 2025-10-08T14:02:23Z

Hi @vaibhav-datazip, just a quick reminder to review my PR when you get a chance!

vaibhav-datazip · 2025-10-08T16:31:28Z

Hi @vaibhav-datazip, just a quick reminder to review my PR when you get a chance!

will review your PR soon @GergesHany

vaibhav-datazip · 2025-10-10T07:59:34Z

LGTM

GergesHany · 2025-10-10T15:24:22Z

LGTM

@vaibhav-datazip Thanks for the approval! I hope proceed with the merge soon

…ame-in-filter

types/stream_configured_test.go

vaibhav-datazip · 2025-10-27T16:23:23Z

types/stream_configured_test.go

+		{
+			name:        "three conditions (not supported)",
+			filter:      "a = 1 and b = 2 and c = 3",
+			expectError: true,
+		},


this case is already added

please remove this as you have already added this case above named too many conditions

types/stream_configured_test.go

…ame-in-filter

Akshay-datazip · 2025-10-30T10:24:28Z

hi @GergesHany feel free to join our slack to take the lead of this pr if you are facing issues
https://join.slack.com/t/getolake/shared_invite/zt-2uyphqf69-KQxih9Gwd4GCQRD_XFcuyw

GergesHany · 2025-11-02T12:33:06Z

@vaibhav-datazip Done

vaibhav-datazip · 2025-11-05T06:40:13Z

@vaibhav-datazip Done

thanks @GergesHany , will be reviewing soon

…ame-in-filter

vaibhav-datazip · 2025-10-30T07:10:38Z

types/stream_configured_test.go

+func TestGetFilterManual(t *testing.T) {
+	testCases := []string{
+		`status = "active"`,
+		`"user-id" > 5`,
+		"`order date` = \"2024-01-01\"",
+		`"user-id" > 5 and ` + "`item.price` <= 100",
+		`age >= 18 or status != "inactive"`,
+	}
+
+	for _, filter := range testCases {
+		cs := &ConfiguredStream{
+			StreamMetadata: StreamMetadata{
+				Filter: filter,
+			},
+		}
+
+		result, err := cs.GetFilter()
+		if err != nil {
+			t.Logf("Filter: %s -> ERROR: %v", filter, err)


we have recently merged unit tests into staging you can see that and make changes to make unit tests consistent , we are using testify for assert

vaibhav-datazip · 2025-11-05T06:51:33Z

types/stream_configured_test.go

+func TestGetFilterManual(t *testing.T) {
+	testCases := []string{
+		`status = "active"`,
+		`"user-id" > 5`,
+		`age >= 18 or status != "inactive"`,
+	}
+
+	for _, filter := range testCases {
+		cs := &ConfiguredStream{
+			StreamMetadata: StreamMetadata{
+				Filter: filter,
+			},
+		}
+
+		result, err := cs.GetFilter()
+		if err != nil {
+			t.Logf("Filter: %s -> ERROR: %v", filter, err)
+		} else {
+			t.Logf("Filter: %s -> Conditions: %+v, LogicalOp: %s", filter, result.Conditions, result.LogicalOperator)
+		}
+	}
+}


Why do we need this manual testing function when you have already added another function which tests get filter ?

vaibhav-datazip · 2025-11-05T06:54:00Z

types/stream_configured_test.go

+import (
+	"strings"
+	"testing"
+)
+


please use testify for checking the test cases. if you have doubt, you can see the following unit test related files

olake/types/catalog_test.go

olake/utils/typeutils/resolver_test.go

olake/utils/typeutils/flatten_test.go

vaibhav-datazip · 2025-11-05T07:04:29Z

types/stream_configured_test.go

+			},
+			expectError: false,
+		},
+		// Supports double-quoted column identifiers (e.g., contains hyphen).


one more test case can be added (if its not already present) , where column name is not passed in quotes

vaibhav-datazip · 2025-11-05T07:07:36Z

types/stream_configured_test.go

+		},
+		// Simple comparison without spaces around operator.
+		{
+			name:   "test case from user: a>b",


why is test case from user: written in some of the test cases, we can give them better names

vaibhav-datazip · 2025-11-05T07:19:51Z

types/stream_configured_test.go

+		{
+			name:        "triple equals with greater than",
+			filter:      "a >=== b",
+			expectError: true,
+		},
+		// Nonsensical operator sequences (four equals with >) should error.
+		{
+			name:        "four equals with greater than",
+			filter:      "a ====> b",
+			expectError: true,
+		},
+		// Nonsensical operator sequences (four equals with <) should error.
+		{
+			name:        "four equals with less than",
+			filter:      "a ====< b",
+			expectError: true,
+		},
+		// Nonsensical operator sequences mixing equals and not-equals should error.
+		{
+			name:        "triple equals with not equal",
+			filter:      "a ===!= b",
+			expectError: true,
+		},
+		// Multiple equals signs (===) are invalid in this grammar.
+		{
+			name:        "multiple equals signs",
+			filter:      "a === b",
+			expectError: true,
+		},
+		// Quadruple equals are invalid and should error.


for these cases, names don't look good, you can use multiple instead of using triple quadruple and also remove redundant looking cases from here as well

vaibhav-datazip · 2025-11-05T07:21:30Z

types/stream_configured_test.go

+			filter:      `"col\"name" = "val\"ue"`,
+			expectError: true,
+		},
+


remove unnecessary line breaks

vaibhav-datazip · 2025-11-05T07:24:34Z

types/stream_configured_test.go

+		// Missing value after '=' should error.
+		{
+			name:        "empty value after operator",
+			filter:      "col = ",
+			expectError: true,
+		},


this kind of test case is already present named missing value

vaibhav-datazip · 2025-11-05T07:26:15Z

types/stream_configured_test.go

+		{
+			name:   "unquoted column with trailing space before op",
+			filter: "col  = 5",
+			expectedFilter: Filter{
+				Conditions: []Condition{
+					{Column: "col", Operator: "=", Value: "5"},
+				},
+				LogicalOperator: "",
+			},
+			expectError: false,
+		},
+	}


this test case is also covered in excessive whitespace

vaibhav-datazip · 2025-11-05T07:27:06Z

types/stream_configured_test.go

+				if actualCondition.Column != expectedCondition.Column {
+					t.Errorf("Condition[%d] Column mismatch: expected %q, got %q", i, expectedCondition.Column, actualCondition.Column)
+				}
+
+				if actualCondition.Operator != expectedCondition.Operator {
+					t.Errorf("Condition[%d] Operator mismatch: expected %q, got %q", i, expectedCondition.Operator, actualCondition.Operator)
+				}
+
+				if actualCondition.Value != expectedCondition.Value {
+					t.Errorf("Condition[%d] Value mismatch: expected %q, got %q", i, expectedCondition.Value, actualCondition.Value)
+				}


instead of these you can use assert from testify library

vaibhav-datazip · 2025-11-05T07:28:29Z

please also check older unresolved comments

…elper

GergesHany · 2025-11-05T08:56:51Z

please also check older unresolved comments

@vaibhav-datazip

All comments have been resolved.
Let me know if there’s anything else needed.

…ame-in-filter

GergesHany changed the base branch from master to staging October 3, 2025 14:14

Merge branch 'staging' into fix/handle-special-characters-in-column-n…

c369f5d

…ame-in-filter

vaibhav-datazip reviewed Oct 4, 2025

View reviewed changes

types/stream_configured.go Outdated Show resolved Hide resolved

GergesHany added 2 commits October 5, 2025 15:30

fix: update regex and add new test cases

ce5816f

fix: Update the regex to NOT accept trailing dots without decimal digits

9185d89

GergesHany requested a review from vaibhav-datazip October 6, 2025 10:00

vaibhav-datazip reviewed Oct 6, 2025

View reviewed changes

types/stream_configured.go Outdated Show resolved Hide resolved

types/stream_configured.go Outdated Show resolved Hide resolved

types/stream_configured.go Outdated Show resolved Hide resolved

GergesHany added 2 commits October 7, 2025 13:44

refactor: remove backtick and <> operator support from filter parser

98f7d5d

Remove unnecessary intermediate variables (operator[1,2], value[1,2])…

83df23c

… and use regex match indices directly in Condition struct initialization.

GergesHany requested a review from vaibhav-datazip October 7, 2025 10:58

vaibhav-datazip added the hacktoberfest Issues open for Hacktoberfest contributors label Oct 7, 2025

Merge branch 'staging' into fix/handle-special-characters-in-column-n…

8f2053d

…ame-in-filter

vaibhav-datazip temporarily deployed to Publish Driver October 14, 2025 09:02 — with GitHub Actions Inactive

vaibhav-datazip temporarily deployed to Publish Driver October 14, 2025 09:03 — with GitHub Actions Inactive

vaibhav-datazip added 2 commits October 19, 2025 13:42

Merge branch 'staging' into fix/handle-special-characters-in-column-n…

ef7c64f

…ame-in-filter

Merge branch 'staging' into fix/handle-special-characters-in-column-n…

6739e9a

…ame-in-filter

vaibhav-datazip reviewed Oct 27, 2025

View reviewed changes

Merge branch 'staging' into fix/handle-special-characters-in-column-n…

306da56

…ame-in-filter

Refactor

b689d4f

GergesHany requested a review from vaibhav-datazip November 2, 2025 12:33

Merge branch 'staging' into fix/handle-special-characters-in-column-n…

ef3ecdc

…ame-in-filter

vaibhav-datazip reviewed Nov 5, 2025

View reviewed changes

GergesHany added 8 commits November 5, 2025 10:25

removed the duplicate 'three conditions (not supported)' test case.

45d2561

refactor: migrate filter tests to testify and remove redundant test h…

eb5ca9a

…elper

test: add test case for unquoted columns with underscores in filter

7c6146b

Updated the test case names to be more descriptive

04d6522

Removed the duplicate 'double quotes mismatch' test case

0788b54

Cleaned up redundant test cases.

70e3f02

Removed the duplicate 'empty value after operator' test case

e5ac061

Removed the duplicate 'unquoted column with trailing space before op'

135081a

GergesHany requested a review from vaibhav-datazip November 5, 2025 08:56

Merge branch 'staging' into fix/handle-special-characters-in-column-n…

a61b0fc

…ame-in-filter

feat: support special characters in column names for filter parsing #557

Are you sure you want to change the base?

feat: support special characters in column names for filter parsing #557

Conversation

GergesHany commented Oct 2, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Uh oh!

CLAassistant commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vaibhav-datazip commented Oct 3, 2025

Uh oh!

GergesHany commented Oct 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GergesHany commented Oct 8, 2025

Uh oh!

vaibhav-datazip commented Oct 8, 2025

Uh oh!

vaibhav-datazip commented Oct 10, 2025

Uh oh!

GergesHany commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Akshay-datazip commented Oct 30, 2025

Uh oh!

GergesHany commented Nov 2, 2025

Uh oh!

vaibhav-datazip commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vaibhav-datazip commented Nov 5, 2025

Uh oh!

GergesHany commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

GergesHany commented Oct 2, 2025 •

edited by cursor bot

Loading

CLAassistant commented Oct 2, 2025 •

edited

Loading

GergesHany commented Oct 10, 2025 •

edited

Loading

vaibhav-datazip commented Nov 5, 2025 •

edited

Loading