-
-
Notifications
You must be signed in to change notification settings - Fork 18
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 #133: Allow some special chars in table name #134
Fix #133: Allow some special chars in table name #134
Conversation
dot(in postgreSql) -(in PostgreSQL/SQLite/SQL Server) with double qoute or with backticks in MySql
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #134 +/- ##
============================================
- Coverage 39.63% 39.59% -0.04%
- Complexity 226 227 +1
============================================
Files 33 34 +1
Lines 873 841 -32
============================================
- Hits 346 333 -13
+ Misses 527 508 -19 ☔ View full report in Codecov by Sentry. |
1f5d7c5
to
5e46cb4
Compare
@@ -24,7 +24,7 @@ public function __construct( | |||
private readonly string $namespace = 'App\\Model', | |||
#[Required] | |||
#[Regex( | |||
pattern: '/^[a-z_][a-z0-9_]*$/i', | |||
pattern: '/^[\w-]+(?:\.[\w-]+)?$/i', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't add it as the test environment isn't set up yet in the repo.
An outside test can be seen here: https://regex101.com/r/aLjS42/1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually there are some bugs: https://regex101.com/r/KX8vqz/1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe kind of this https://regex101.com/r/B8gCXv/1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also may be several dots. For example, in MSSQL: database_name.schema_name.table_name
.
The table name specification is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to check only symbols, for example, /^[\w-"
]+$/`.
And do a more detailed check later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to check only symbols, for example,
/^[\w-"
]+$/`. And do a more detailed check later.
I'm also of the same opinion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know which one I should use.
^(["'`]?)([\w][\w-]*)\1(?:\.((?1))(?2)\3)*$
https://regex101.com/r/3zbKmI/1
OR
^[\w\-."`]+$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaik -
is not permitted to use, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://dev.mysql.com/doc/refman/8.0/en/identifiers.html
tested in MySql, it's permitted
Co-authored-by: Sergei Predvoditelev <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code itself looks good 👍
Thank you! |
Fixes #133
Table name regex correction
Exception handling for syntax error when passing a wrong table name
Regex test: https://regex101.com/r/QmSWaW/1