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

Discussion: Validation of SQL #1005

Closed
gitmotte opened this issue Jun 30, 2020 · 7 comments
Closed

Discussion: Validation of SQL #1005

gitmotte opened this issue Jun 30, 2020 · 7 comments

Comments

@gitmotte
Copy link
Contributor

gitmotte commented Jun 30, 2020

Since there are multiple databases supported it would be possible to contruct or parse SQL which is not executable against those databases. I'm missing a concept to distinguish them.

I found some Prefixed classes like OracleHint, some flags like PlainSelect.mySqlHintStraightJoin, ...

What if we add a ValidationVisitor to the library - which could be used to check the parsed/build sql for validity of a specific database.

gitmotte added a commit to gitmotte/JSqlParser that referenced this issue Jun 30, 2020
@wumpz
Copy link
Member

wumpz commented Jul 1, 2020

That would be indeed a benefit. And since JSqlParsers core of being multi database capable will not change the implementation this as a visitor seems the right way to go. Additional I want to add here some points that could be validated as well besides the compatibility with specific database systems:

  • validate against a database schema (used columns, tables, ...)
  • validate scope of selected objects (a database schema needs to be there)
    ** and build a relationbetween columns and tables (select a,b from mytab1, mytab2)

@gitmotte
Copy link
Contributor Author

gitmotte commented Jul 1, 2020

Validation against database metadata is a second step, surely. But there is not always a schema present and we should not relate on it.
Btw. Fetching metadata is also database - specific.

If a connection is present we can test the Statement against it and let the database validate those things.

@gitmotte
Copy link
Contributor Author

gitmotte commented Jul 4, 2020

I did a small improvement on deparser-api during analyses - hope its a step forward ...
#1007

gitmotte added a commit to gitmotte/JSqlParser that referenced this issue Jul 12, 2020
gitmotte added a commit to gitmotte/JSqlParser that referenced this issue Jul 16, 2020
@gitmotte
Copy link
Contributor Author

gitmotte commented Jul 16, 2020

That would be indeed a benefit. And since JSqlParsers core of being multi database capable will not change the implementation this as a visitor seems the right way to go.

@wumpz
I thought about your statement that the core will not change to be multi database capable.
This could end up in a grammar which is not maintainable any more. I thought about a pluggable system to add own constructs, extension points for non-standard syntax or multiple database specific parsers making the validation for a database obsolete, because the parser cannot parse syntax valid within other databases and the model could be specific to each database too. Interfaces can force common model-fields and other users are able to add extensions by adding a library to the classpath. A factory can be used to choose the correct parser. This change can be on top of a multi database capable parser variant, which could be replaced step by step by a parser trying to parse against a set of parsers for probing a syntax.

@wumpz
Copy link
Member

wumpz commented Jul 16, 2020

I thought also about maintaining multiple grammars but decided against it, at least for now. Within the bracket discussion that was a point as well. Now The parser has a flag, to switch something on or off.

Assume you go the multigrammar way. Where would you make the cut? Even between the versions of one database are differences. Or is there a way if plugging two different grammars together or build a grammar up out of even smaller grammar blocks?

Imho the multidatabase part is one of the key benefits of this parser.

@gitmotte
Copy link
Contributor Author

gitmotte commented Jul 16, 2020

I thought also about maintaining multiple grammars but decided against it, at least for now. Within the bracket discussion that was a point as well. Now The parser has a flag, to switch something on or off.

Assume you go the multigrammar way. Where would you make the cut? Even between the versions of one database are differences. Or is there a way if plugging two different grammars together or build a grammar up out of even smaller grammar blocks?

Imho the multidatabase part is one of the key benefits of this parser.

we could combine those things by

  • adding on/off switches for features and/or a set of features defined new within a new database-version (or outdated by this version)
  • and make a cut for other database-systems by providing a new grammar.

yes, most database-systems share a common syntax based on sql-92, sql-96, .. that would produce a lot of code which is duplicated, but without the downside to impact all parsers. Unfortunately I do not know at the moment a system plugging multiple grammars together - seems there are no include/override mechanisms in java-cc (or antlr4) (?)

probably a database-system could be a feature-set too, i.e.

  • postgresql (all features of all versions?)
    • postgresql12 extends postgresql11
      • feature-x=off
      • feature-y=new
    • postgresql11
      • feature-x=deprecated
  • mysql
    • mysql8
      • feature-y=new
  • feature-x (mysql8, postgresql11, postgresql12, ... )
  • feature-y

@gitmotte
Copy link
Contributor Author

gitmotte commented Jul 17, 2020

@wumpz please have a look at newest proposal for

  • an extensible validation visitor framework (ValidationCapability and its subtypes FeatureSetValidation, DatabaseMetaDataValidation, AllowedTypesValidation)
  • FeatureConfiguration/Feature/FeatureSet class

branch https://github.com/gitmotte/JSqlParser/tree/master.validate within packages

  • net.sf.jsqlparser.util.validation
  • net.sf.jsqlparser.parser.feature

gitmotte added a commit to gitmotte/JSqlParser that referenced this issue Aug 26, 2020
its subtypes FeatureSetValidation, DatabaseMetaDataValidation,
AllowedTypesValidation)
* FeatureConfiguration/Feature/FeatureSet class

new packages
* net.sf.jsqlparser.util.validation
* net.sf.jsqlparser.parser.feature

------

history:
* fresh clone from JSQLParser/JSqlParser (master) -> master.validate
* (Merge the work on this issue done in 'github.validate' into
master.validate)

JSQLParser#1005
wumpz pushed a commit that referenced this issue Nov 6, 2020
* * add with prefix for fluent setters.

 #1004

* add getters

* * add with prefix for fluent setters. (revert to chaining setters, do
not break current api)

 #1004

* * add with prefix for fluent setters. (revert to chaining setters, do
not break current api)

 #1004

* use new methods within testcases

* use new methods within testcases

* use new methods within testcases

* use new methods within testcases

* use new methods within testcases

* use new methods within testcases

* use new methods within testcases

* use new methods within testcases

* remove create() methods - they do not add enough value to be justified

* * use new methods within testcases
* add some constructors
* fix and add "with" / "add" methods

* * use new methods within testcases

* * use new methods within testcases
* add some constructors

* * renamed constant

* use new methods within testcases

* use new methods within testcases

* use new methods within testcases

* use new methods within testcases

* * use new methods within testcases
* add some with-methods
* add getter/setter named after the field without abbrivation

* * use new methods within testcases

* remove empty implicit constructor

* return the deparsed Statement - object

* compare object tree

* compare object tree

* * fix ObjectTreeToStringStyle
* compare object tree

* remove casts not needed

* * use new methods within testcases
* add some "set" "with" "add" methods missing

* * use new methods within testcases

* add empty constructors and override with-/add-methods returning concrete
type

* * add ReflectionModelTest

* * use new methods within testcases

* fix checkstyle errors

* license header

* remove test-classes from ReflectionModelTest

* remove visitoradapter-classes from ReflectionModelTest

* * add SelectDeParser(StringBuilder)
* remove overriding setters/getters of buffer
#1007

* push to synbee-contrib

org.synbee.commons.contrib:jsqlparser:3.2-0.0.6-SNAPSHOT

* add ValidationUtil for simple validation of one or more statements

* remove overrides of
* getCause
* printStackTrace variants

why add an additional cause ?

set cause.getMessage() the message within constructor
JSQLParserException(Throwable cause), othewise cause.toString() will be
set as default.

* add ValidationVisitor showcase
#1005

* add ValidationUtil for simple validation of one or more statements

* remove overrides of
* getCause
* printStackTrace variants

why add an additional cause ?

set cause.getMessage() the message within constructor
JSQLParserException(Throwable cause), othewise cause.toString() will be
set as default.

* visit(ShowTablesStatement)

* copyright/license

* add stubs (use deparsers as template)

* Merge branch 'master.validate' of
https://github.com/gitmotte/JSqlParser.git into master.validate

* add ValidationVisitor showcase
#1005

* add ValidationUtil for simple validation of one or more statements

* remove overrides of
* getCause
* printStackTrace variants

why add an additional cause ?

set cause.getMessage() the message within constructor
JSQLParserException(Throwable cause), othewise cause.toString() will be
set as default.

* visit(ShowTablesStatement)

* add stubs (use deparsers as template)

* Merge branch 'master.validate' of
https://github.com/gitmotte/JSqlParser.git into master.validate

* add tests for ValidationUtil

* + implements OrderByVisitor

* split Expressionvalidator which implements both ItemsListVisitor and
Expressionvisitor into Expressionvalidator and ItemListValidator

* Merge branch 'github.validate'

* implement upsertvalidator

* add copyright

* validate through given ValidationCapability's

* * switch to new method forced by
ValidationCapability.validate(ValidationContext context,
Consumer<String> errorMessageConsumer);
* add AllowedTypesValidation

* add FeatureConfiguration

* use FeatureConfiguration within parser

* repair pom.xml

* repair pom.xml

* repair pom.xml

* repair pom.xml

* * make FeatureConfiguration not a singleton any more
* CCJSqlParser extends AbstractJSqlParser<CCJSqlParser>
* add FeaturesAllowed for testing against features allowed

* implement some Validators

* basic implementation of DatabaseMetaDataValidation /
JdbcDatabaseMetaDataCapability

* moving classes to sub-packages

* * moving classes to sub-packages
* fixing some bugs

* repair pom.xml

* add and fix validations

* add javadoc

* * force definition of ```public String getMessage(Feature feature)```
in FeatureSetValidation
* allow all objects as feature-value - this may be needed by the parser,
if a none-boolean configuration is needed

* impl.
* SelectValidator.visit(PlainSelect)
* OrderByValidator

* add Version-enums

* impl.
* InsertValidator
* multiple implementations of visit(SubSelect) -> forward to
SelectValidator
* add some known features to SqlServerVersion

* refactoring enum-name should be upper case

* add ansi sql enum

* refactoring enum-name should be upper case

* implement limitvalidator

* + validateOffset

* + validateFetch

* + validate Pivot, UnPivot, PivotXml

* + implement DropValidator

* change testcase to image a more probably usecase

* * add javadoc and
* predefined sets for EXECUTE, ALTER, DROP
* allow to combine FeatureSets

* * implement executevalidator

* implement ExpressionValidator

* implement GrantValidator

* javadoc and complete SELECT constant

* use utility methods from AbstractValidator

* more user friendly names

* javadoc

* add subtypes for ValidationException
* ValidationParseException
* DatabaseException
* UnexpectedValidationException
and change Set<String> errors to Set<ValidationException> for collect.

* javadoc & rename exception

* rename method

* extract parsing task into package - private class for {@link
ValidationUtil} to parse the statements
 * within it's own {@link ValidationCapability}

* add null-check for parsedStatement

* bugfix - do not collect duplicates

* implement toString() for
* ValidationError
* ValidationException

* add simple caching

* + validateOptionalFromItem(s)

* * implement GroupByValidator

* implement merge-validator

* renaming ItemListValidator -> ItemsListValidator

* + validateOptionalItemsList
+ implement ReplaceValidator
+ use validateOptionalColumns, validateOptionalExpression where possible

* * remove validateOptionalColumns -> switch to
validateOptionalExpressions
* move validateOptionalOrderByElements to AbstractValidator
* add validateOptional in AbstractValidator
* add validateOptionalList in AbstractValidator

* + SetStatementValidator

* + ValuesStatementValidator

* + UseStatementValidator

* * implement UpdateValidator

* * implement ShowStatementValidator/ShowColumnsStatementValidator

* * implement UpdateValidator

* * add Feature.jdbcParameter, Feature.jdbcNamedParameter, to all
featuresets
* + Version.getFeaturesClone
* add javadoc to Version-enum-constructors

* + validateOptionalFeature

* * implement DeleteValidator

* ...

* fix typo

* small optimization

* * move method getFeaturesClone to FeatureSet
* implement join - validation
* add copy(), add(Collection), remove(*) methods to FeaturesAllowed

* * add join - features to sqlserver, h2

* implementations

* bugfix - merging the errors

* copyright

* #1022

* add more fine granular control for setOperations

* fix nullpointerexception

* add more fine granular control for comments

* add Features supported

* * add javadoc
* add features to *Version-files

* extract methods isNotEmpty

* check for isNotEmpty

* * add features to *Version-files

* always parse net.sf.jsqlparser.statement.Statements and validate the
list of included net.sf.jsqlparser.statement.Statement's

* add known mariadb features

* new names-set for FeaturesAllowed

* new names-set for FeaturesAllowed

* new names-set for FeaturesAllowed

* add ature.withItem, Feature.withItemRecursive to H2

* Feature.setOperation, Feature.setOperationUnion,
Feature.setOperationIntersect, Feature.setOperationExcept,

                    for MariaDb

* add features to SQLServer

* Merge branch 'master.orig' into github.validate

* @OverRide() -> @OverRide

* fix typing error "joinStaight" > joinStraight

* rename Feature "insertValues" -> "values" and use "insertValues" for
INSERT INTO ... VALUES

* add javadoc

* add Feature.selectGroupByGroupingSets to PostgresqlVersion

* implement basic OracleVersion

* add Feature.mySql* - also supported by mariadb

* add some more finegraned control over "drop" Feature.
* drop,
* dropTable,
* dropIndex,
* dropView,
* dropSchema,
* dropSequence,
* dropIfExists,

* complete FeaturesAllowed groups INSERT/UPDATE/DELETE/MERGE/DML

* add link to documentation

* fix - duplicate use of feature "function" - the use of functions in
statements and "createFunction" as a ddl statement

* TODO this feature seams very close to a jsqlparser-user usecase

* * implement MySqlVersion
* replace feature Feature.dropIfExists by features dropTableIfExists,
dropIndexIfExists, dropViewIfExists, dropSchemaIfExists,
dropSequenceIfExists
* add methods FeatureSet.getNotContained FeatureSet.retainAll

* remove HSQLDBVersion - do not support this variant

* remove HSQLDBVersion - do not support this variant

* add unit-test

* + add unittests for
* UpdateValidator
* DeleteValidator
add stubs for all other Validator-classes

+ ModifyableFeatureSet

* add some utility-methods in ValidationTestAsserts

* complete unit-tests for InsertValidator

* remote Feature.insertReturningExpressionList for Oracle -
returning_clause requires INTO clause (only PL/SQL)

* add some more select validation tests

* add DropValidatorTests

* add DropValidatorTests

* add CreateTableValidatorTests

* add CreateTableValidatorTests

* add ExpressionValidatorTests

* add OrderByValidatorTest

* use isNotEmpty

* implement GroupByValidatorTest

* implement CreateSequenceValidatorTest

* remove @ignore - test is ok

* implement CreateIndexValidatorTest

* implement CreateViewValidatorTest

* enable validation of Feature.commentOnView (#1024 is merged already)

* change format of #toString() for better readability

* * implement MergeValidatorTest
* implement ReplaceValidatorTest
* implement StatementValidatorTest

* rename
* ValidationUtil -> Validation
* ValidatorUtil -> ValidationUtil
add testcases for ValidationUtil

* add DatabaseMetaDataValidationTest

* checkstyle fix

* add copyright statement

* add unit-tests for show tables, show column, show statements

* * add ExecuteValidatorTest
* as there is a difference between execute <procedure> and execute
[immediate] <dynamic sql> with USING expr, ... remove support for
execute on MYSQL, MARIADB, ORACLE

* * add ExecuteValidatorTest for CALL fnName (mysql, mariadb, postgres)

* add upsertvalidatortest

* add GrantValidatorTest

* add AlterSequenceValidatorTest

* add AlterSequenceValidatorTest

* add AlterViewValidatorTest

* add AlterValidatorTest

* replace != null by isNotEmpty on collections

* fix formatting

* add validate commit

* add validate block

* add DeclareStatementValidatorTest

* let NamesLookup implement UnaryOperator<String>

* let NamesLookup implement UnaryOperator<String>

* add javadoc

* add more DatabaseMetaDataValidationTest's

* extract JdbcDatabaseMetaDataCapability.splitAndValidateMinMax

* add pivot/unpivot/pivotxml validation testcases

* add testcase for Feature.tableFunction

* add test for lateral joins and subjoins

* add testValidationRowMovementOption

* add values validator test

* move tests to LimitValidatorTest

* move tests to UseStatementValidatorTest

* add tests for SET - statements

* fix checkstyle error

* new serialVersionUID

* add validation for NamedObject not existing

* need table/view reference to validate column names

* fix typo

* fix errormessage (Arrays.toString(types))

* add trigger, alias
return null, instead of throwing exception, if not found

* extract NamesLookup to own file (jdk-bug enum inner classes)

* fix name-check AlterOperation.ALTER

* fix error message

* remove methods not needed (they only delegate to ValidationContext)

* add tests - validate metadata

* fix compile error

* fix columnExists check - depending on the statement the prefix is an
alias, a table/view or it has no prefix (need to lookup within all
related tables/views)

* fix javadoc warnings
@wumpz wumpz closed this as completed Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants