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

fix #1106 #CORE-3269 degree sign accepted as text #1107

Closed
wants to merge 5 commits into from
Closed

fix #1106 #CORE-3269 degree sign accepted as text #1107

wants to merge 5 commits into from

Conversation

matteoturra
Copy link

@matteoturra matteoturra commented Apr 22, 2020


name: Pull Request
about: Create a report to help us improve
title: ''
labels: Status:Discovery
assignees: ''


Environment

Liquibase Version: 3.8.6

Database Vendor & Version: Oracle 12

Operating System Type & Version: Windows 10

Pull Request Type

  • [ x] 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

The sqlformat parser does not accepts certain character from Latin1 Iso extension (like £°) throwing TokenMgrError
See: https://liquibase.jira.com/browse/CORE-3269

Steps To Reproduce

Create a changeset with an insert statement like
insert into table (column) values ('warm temp. 32°')

See #1106

Fast Track PR Acceptance Checklist:

  • [ x] Build is successful and all new and existing tests pass
  • [ x] Added Unit Test(s)
  • Added Integration Test(s)
  • Documentation Updated

@matteoturra matteoturra changed the title fix 1106 - CORE-3269 degree sign accepted as text fix #1106 - CORE-3269 degree sign accepted as text Apr 22, 2020
@matteoturra matteoturra changed the title fix #1106 - CORE-3269 degree sign accepted as text fix #1106 #CORE-3269 degree sign accepted as text Apr 22, 2020
@ro-rah
Copy link

ro-rah commented Apr 24, 2020

Hi @matteoturra ,

Many thanks for your PR submission!

Would you mind filling out the description template for this PR? We use this to assess the PRs before they enter into internal Engineering. It looks like these items like description of the issue being fixed, why you updated the files, version of Liquibase, and steps to reproduce would be helpful to. This is what step 9 details in our community guide to submitting PRs.

Thanks for creating unit test!

Again, thanks for all the work you have put in so far.

-Ronak

@ro-rah
Copy link

ro-rah commented Apr 27, 2020

Hi @matteoturra ,

Thanks for writing the Unit tests, and I see this should close #1106 as well as closes CORE-3269. The unit test speaks for itself and the test can be the following insert should not cause a parse error:
insert into table (column) values ('warm temp. 32°')

I'll get this into conditioning.

@ro-rah ro-rah added RiskLow Trivial changes in spelling, documentation changes, focused bug fixes, etc. Sprint7.4 and removed StatusDiscovery labels Apr 27, 2020
@ro-rah
Copy link

ro-rah commented Apr 27, 2020

Reasoned on the Risk low b/c it contains unit tests, and it is currently broken.

@Aeolun
Copy link

Aeolun commented Jan 6, 2021

@ro-rah It's been months and it appears this is still an issue. I'm currently unable to deal with fullwidth tilde ~. Is there any chance to get this merged?

@Aeolun
Copy link

Aeolun commented Jan 6, 2021

There are actually a lot more ranges that break raw SQL when you are working with the Japanese language.

image

@ro-rah
Copy link

ro-rah commented Jan 6, 2021

Hi @Aeolun and @matteoturra, escalating to my colleague @molivasdat for update.

@kataggart kataggart added this to To Do in Conditioning++ via automation Dec 27, 2021
@liquibase liquibase deleted a comment from codecov bot Jan 5, 2022
@nvoxland
Copy link
Contributor

nvoxland commented Jan 5, 2022

This change was already made at some point, but I created #2329 for the additional unicode points @Aeolun mentioned

@nvoxland nvoxland closed this Jan 5, 2022
Conditioning++ automation moved this from To Do to Done Jan 5, 2022
@kataggart
Copy link
Contributor

Thank you @nvoxland !

@nvoxland nvoxland removed this from Done in Conditioning++ Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DBMSSQLServer IntegrationAny RiskLow Trivial changes in spelling, documentation changes, focused bug fixes, etc. ThemeDBObjects TypeBug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants