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

Add support for \u2116 symbol win1251 cyrillic symbol of number #1324

Merged
merged 2 commits into from
Aug 31, 2022

Conversation

Stuchalin
Copy link
Contributor

@Stuchalin Stuchalin commented Aug 14, 2020

Add \u2116 symbol
That is cyrillic number symbol


name: Pull Request FIX https://liquibase.jira.com/browse/CORE-3326
about: this PR fixes bug https://liquibase.jira.com/browse/CORE-3326 Lexical error at line 5, column 16. Encountered: "\u2116" (8470), after : "". "\u2116" is a win1251 cyrillic symbol of number (https://liquibase.jira.com/browse/CORE-3326)
title: 'CORE-3326 - Lexical error at line 5, column 16. Encountered: "\u2116" (8470), after : ""'
labels: Status:Discovery
assignees: ''


Environment

Liquibase Version: 3.5.5, 3.6.0, 3.6.1, 3.6.2, 4.0.1

Liquibase Integration & Version: maven

Liquibase Extension(s) & Version: -

Database Vendor & Version: Oracle 10g

Operating System Type & Version: macOS, Windows 10

Pull Request Type

  • Bug fix (non-breaking change which fixes an issue.)
  • Enhancement/New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

https://liquibase.jira.com/browse/CORE-3326

Fast Track PR Acceptance Checklist:

Add \u2116 symbol
That is cyrillic number symbol
@molivasdat
Copy link
Contributor

Thanks @Stuchalin for your pull request!

Here’s what happens next:

A member of the Liquibase team will take a look at your contribution and may suggest:

The PR will be prioritized according to our internal development and testing capacity.

We’ll let you know when it’s ready to move to the next step or if any changes are needed.

@molivasdat molivasdat added DBOracle RiskMedium Changes that require more testing or that affect many different code paths. Severity3 TypeBug ImpactMedium labels Aug 17, 2020
@kataggart kataggart changed the title Fix CORE-3326 Add support for \u2116 symbol win1251 cyrillic symbol of number Aug 11, 2022
Copy link
Contributor

@nvoxland nvoxland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review and test results:

Things to be aware of:

  • Change makes sense
  • No need for a test since we don't test every letter in the list

Things to worry about:

  • Nothing

@nvoxland nvoxland added the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Aug 30, 2022
@liquibase liquibase deleted a comment from codecov bot Aug 30, 2022
@github-actions
Copy link

Unit Test Results

  4 620 files  ±0    4 620 suites  ±0   34m 52s ⏱️ - 2m 18s
  4 617 tests ±0    4 402 ✔️ +4     215 💤  - 4  0 ±0 
54 576 runs  ±0  49 556 ✔️ +4  5 020 💤  - 4  0 ±0 

Results for commit 1a43982. ± Comparison against base commit e742a88.

Copy link
Contributor

@XDelphiGrl XDelphiGrl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix adds \u2116 to the SimpleSqlGrammar list to allow JavaCC to correctly parse the character.
  • No additional testing required.

APPROVED

@nvoxland nvoxland merged commit 69ef15d into liquibase:master Aug 31, 2022
@kataggart kataggart added this to the NEXT milestone Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autocandidate DBOracle IntegrationMaven RiskMedium Changes that require more testing or that affect many different code paths. SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions Severity3 TypeBug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

8 participants