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

GREL string concatentation (+) overly aggressive #6341

Closed
tfmorris opened this issue Feb 2, 2024 · 0 comments · Fixed by #6342
Closed

GREL string concatentation (+) overly aggressive #6341

tfmorris opened this issue Feb 2, 2024 · 0 comments · Fixed by #6342
Assignees
Labels
grel The default expression language, GREL, could be improved in many ways! Priority: Low Indicates less critical issues that can be dealt with at a later stage Type: Bug Issues related to software defects or unexpected behavior, which require resolution.
Milestone

Comments

@tfmorris
Copy link
Member

tfmorris commented Feb 2, 2024

Currently the string concatenation operator (+) is used in cases where it isn't appropriate so that, for example "1/1/1990".toDate() + 1 converts the values to strings and concatenates them, which is almost certainly not what the user expects.

Current Results

The binary addition operator (+) results in string concatenation when used with anything except two numbers which is overly greedy behavior.

Expected Behavior

String concatenation should be limited to cases where at least one operand is a string. Unsupported cases (e.g. date + integer) should be an error (which just returns null in current GREL conventions).

@tfmorris tfmorris added Type: Bug Issues related to software defects or unexpected behavior, which require resolution. Priority: Low Indicates less critical issues that can be dealt with at a later stage grel The default expression language, GREL, could be improved in many ways! labels Feb 2, 2024
tfmorris added a commit to tfmorris/OpenRefine that referenced this issue Feb 2, 2024
- restrict string concatenation (+) to cases where
  at least one operand is a string (to avoid things like
  concatenating two dates as strings)
- Add support for dates and strings to comparison operators
  Strings use a collator for the default locale with
  normalized decomposition for the comparisons.
- Above implementation uses Comparable, so any future
  data types that implement that interface should get
  supported for free
- Add a bunch more tests
@tfmorris tfmorris self-assigned this Feb 2, 2024
@tfmorris tfmorris added this to the 3.8 milestone Feb 2, 2024
tfmorris added a commit to tfmorris/OpenRefine that referenced this issue Feb 2, 2024
- restrict string concatenation (+) to cases where
  at least one operand is a string (to avoid things like
  concatenating two dates as strings)
- Add support for dates and strings to comparison operators
  Strings use a collator for the default locale with
  normalized decomposition for the comparisons.
- Above implementation uses Comparable, so any future
  data types that implement that interface should get
  supported for free
- Add a bunch more tests
tfmorris added a commit that referenced this issue Feb 14, 2024
- restrict string concatenation (+) to cases where
  at least one operand is a string (to avoid things like
  concatenating two dates as strings)
- Add support for dates and strings to comparison operators
  Strings use a collator for the default locale with
  normalized decomposition for the comparisons.
- Above implementation uses Comparable, so any future
  data types that implement that interface should get
  supported for free
- Add a bunch more tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grel The default expression language, GREL, could be improved in many ways! Priority: Low Indicates less critical issues that can be dealt with at a later stage Type: Bug Issues related to software defects or unexpected behavior, which require resolution.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant