From 16ce451d195534fb172cb29dffc337b997d62102 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Tue, 13 Feb 2024 14:24:49 +0000 Subject: [PATCH 01/15] WIP --- src/macaw/core.clj | 68 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 4 deletions(-) diff --git a/src/macaw/core.clj b/src/macaw/core.clj index 369fe70..4717e71 100644 --- a/src/macaw/core.clj +++ b/src/macaw/core.clj @@ -1,13 +1,20 @@ (ns macaw.core (:import + (net.sf.jsqlparser + Model) (net.sf.jsqlparser.parser CCJSqlParserUtil) + (net.sf.jsqlparser.schema + Column) (net.sf.jsqlparser.statement - Statement) + Statement + StatementVisitor) + (net.sf.jsqlparser.statement.update + Update) (net.sf.jsqlparser.util TablesNamesFinder))) -(set! *warn-on-reflection* true) +#_(set! *warn-on-reflection* true) (defn query->tables "Given a parsed query (i.e., a subclass of `Statement`) return a list of fully-qualified table names found within it. @@ -17,16 +24,53 @@ (let [table-finder (TablesNamesFinder.)] (.getTableList table-finder parsed-query))) +(defprotocol ConjColumn + (conj-column! [x known-columns])) + +(extend-protocol ConjColumn + Model + (conj-column! [_ _known-column-names] + (println "nothing to add") + nil) + + Column + (conj-column! [^Column column known-column-names] + (println "CONJing!") + (swap! known-column-names conj (.getColumnName column)))) + (defn query->columns "TODO: implement!" - [^Statement _parsed-query] - ["oh no" "TODO"]) + [^Statement parsed-query] + (println "query->columns") + (let [column-names (atom []) + column-finder (proxy + [TablesNamesFinder] + [] + (visit [visitable] + (println "visiting") + (println {:visitable visitable + :column-names @column-names}) + (let [^TablesNamesFinder this this] + (conj-column! visitable column-names) + (proxy-super visit visitable))))] +#_ (.init column-finder false) +#_ (.accept parsed-query column-finder) + + (.getTables column-finder parsed-query) + #_ @column-names) +) + (defn parsed-query "Main entry point: takes a string query and returns a `Statement` object that can be handled by the other functions." [^String query] (CCJSqlParserUtil/parse query)) +(-> "select foo, bar from baz;" + parsed-query + query->columns) + + (defn resolve-columns "TODO: Make this use metadata we know about. TODO: If nil is a column (from a select *) then no need for the rest of the entries @@ -46,3 +90,19 @@ tables (query->tables parsed) columns (query->columns parsed)] (resolve-columns tables columns))) + + + + + +(comment + + + + @(u/prog1 (atom []) + (conj-column! 1 <>) + (conj-column! 2.0 <>) + (conj-column! (Integer. 8) <>)) + + + ) From 054f5e2daba0d02b45832adbb30451f79b02fca5 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Tue, 13 Feb 2024 16:02:11 +0000 Subject: [PATCH 02/15] Set up build for Java --- README.md | 20 ++++++++++++++++++++ build.clj | 47 +++++++++++++++++++++++++++++++++++++++++++++++ deps.edn | 6 +++--- 3 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 build.clj diff --git a/README.md b/README.md index c4e880a..bca7784 100644 --- a/README.md +++ b/README.md @@ -11,3 +11,23 @@ Macaw is a limited Clojure wrapper for [JSqlParser](https://github.com/JSQLParser/JSqlParser). Similar to its parrot namesake, it's intelligent, can be taught to speak SQL, and has many colors (supports many dialects). + + +## Building + +To build a local JAR, use + +``` +clj -T:build jar +``` + +This will create a JAR in the `target` directory. + +The build process is slightly complicated since Macaw mixes Clojure and Java +files. If you're working on Macaw itself and make changes to a Java file, you +must: + +1. Rebuild +2. Restart your Clojure REPL + +for the changes to take effect. diff --git a/build.clj b/build.clj new file mode 100644 index 0000000..17ff3fe --- /dev/null +++ b/build.clj @@ -0,0 +1,47 @@ +;; Further info: https://clojure.org/guides/tools_build#_mixed_java_clojure_build + +(ns build + (:require + [clojure.java.shell :as sh] + [clojure.string :as str] + [clojure.tools.build.api :as b])) + +(def lib 'metabase/macaw) + +(def major-minor-version "0.1") + +(defn commit-number [] + (or (-> (sh/sh "git" "rev-list" "HEAD" "--count") + :out + str/trim + parse-long) + "9999-SNAPSHOT")) + +(def version (str major-minor-version \. (commit-number))) +(def target "target") +(def class-dir (format "%s/classes" target)) + +(def jar-file (format "target/%s-%s.jar" (name lib) version)) + +(def basis (delay (b/create-basis {:project "deps.edn"}))) + +(defn clean [_] + (b/delete {:path target})) + +(defn compile [_] + (b/javac {:src-dirs ["java"] + :class-dir class-dir + :basis @basis + :javac-opts ["--release" "11"]})) + +(defn jar [_] + (compile nil) + (b/write-pom {:class-dir class-dir + :lib lib + :version version + :basis @basis + :src-dirs ["src"]}) + (b/copy-dir {:src-dirs ["src" "resources"] + :target-dir class-dir}) + (b/jar {:class-dir class-dir + :jar-file jar-file})) diff --git a/deps.edn b/deps.edn index 01bbb86..01557ac 100644 --- a/deps.edn +++ b/deps.edn @@ -1,5 +1,5 @@ {:paths - ["src" "resources"] + ["src" "java" "resources"] :deps {com.github.jsqlparser/jsqlparser {:mvn/version "4.8"}} ; The actual SQL Parser to wrap! @@ -41,8 +41,8 @@ {:deps {com.github.camsaul/whitespace-linter {:sha "e35bc252ccf5cc74f7d543ef95ad8a3e5131f25b"}} :ns-default whitespace-linter :exec-fn whitespace-linter/lint - :exec-args {:paths ["deps.edn" "src" "test" ".github"] - :include-patterns ["\\.clj[cs]?$" "\\.edn$" "\\.yaml$" "\\.md$"]}} + :exec-args {:paths ["deps.edn" "src" "java" "test" ".github"] + :include-patterns ["\\.clj[cs]?$" "\\.edn$" "\\.java$" "\\.yaml$" "\\.md$"]}} ;; Run tests ;; From 68065587eae453e66741c604ce1074d90283e4f0 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Tue, 13 Feb 2024 16:03:30 +0000 Subject: [PATCH 03/15] Add ASTWalker.java! --- java/com/metabase/macaw/ASTWalker.java | 1207 ++++++++++++++++++++++++ 1 file changed, 1207 insertions(+) create mode 100644 java/com/metabase/macaw/ASTWalker.java diff --git a/java/com/metabase/macaw/ASTWalker.java b/java/com/metabase/macaw/ASTWalker.java new file mode 100644 index 0000000..63bb1f7 --- /dev/null +++ b/java/com/metabase/macaw/ASTWalker.java @@ -0,0 +1,1207 @@ +package com.metabase.macaw; + +// Borrows substantially from JSqlParser's TablesNamesFinder + +import java.util.List; +import java.util.Map; + +import net.sf.jsqlparser.JSQLParserException; +import net.sf.jsqlparser.expression.AllValue; +import net.sf.jsqlparser.expression.AnalyticExpression; +import net.sf.jsqlparser.expression.AnyComparisonExpression; +import net.sf.jsqlparser.expression.ArrayConstructor; +import net.sf.jsqlparser.expression.ArrayExpression; +import net.sf.jsqlparser.expression.BinaryExpression; +import net.sf.jsqlparser.expression.CaseExpression; +import net.sf.jsqlparser.expression.CastExpression; +import net.sf.jsqlparser.expression.CollateExpression; +import net.sf.jsqlparser.expression.ConnectByRootOperator; +import net.sf.jsqlparser.expression.DateTimeLiteralExpression; +import net.sf.jsqlparser.expression.DateValue; +import net.sf.jsqlparser.expression.DoubleValue; +import net.sf.jsqlparser.expression.Expression; +import net.sf.jsqlparser.expression.ExpressionVisitor; +import net.sf.jsqlparser.expression.ExtractExpression; +import net.sf.jsqlparser.expression.Function; +import net.sf.jsqlparser.expression.HexValue; +import net.sf.jsqlparser.expression.IntervalExpression; +import net.sf.jsqlparser.expression.JdbcNamedParameter; +import net.sf.jsqlparser.expression.JdbcParameter; +import net.sf.jsqlparser.expression.JsonAggregateFunction; +import net.sf.jsqlparser.expression.JsonExpression; +import net.sf.jsqlparser.expression.JsonFunction; +import net.sf.jsqlparser.expression.JsonFunctionExpression; +import net.sf.jsqlparser.expression.KeepExpression; +import net.sf.jsqlparser.expression.LongValue; +import net.sf.jsqlparser.expression.MySQLGroupConcat; +import net.sf.jsqlparser.expression.NextValExpression; +import net.sf.jsqlparser.expression.NotExpression; +import net.sf.jsqlparser.expression.NullValue; +import net.sf.jsqlparser.expression.NumericBind; +import net.sf.jsqlparser.expression.OracleHierarchicalExpression; +import net.sf.jsqlparser.expression.OracleHint; +import net.sf.jsqlparser.expression.OracleNamedFunctionParameter; +import net.sf.jsqlparser.expression.OverlapsCondition; +import net.sf.jsqlparser.expression.Parenthesis; +import net.sf.jsqlparser.expression.RangeExpression; +import net.sf.jsqlparser.expression.RowConstructor; +import net.sf.jsqlparser.expression.RowGetExpression; +import net.sf.jsqlparser.expression.SignedExpression; +import net.sf.jsqlparser.expression.StringValue; +import net.sf.jsqlparser.expression.TimeKeyExpression; +import net.sf.jsqlparser.expression.TimeValue; +import net.sf.jsqlparser.expression.TimestampValue; +import net.sf.jsqlparser.expression.TimezoneExpression; +import net.sf.jsqlparser.expression.TranscodingFunction; +import net.sf.jsqlparser.expression.TrimFunction; +import net.sf.jsqlparser.expression.UserVariable; +import net.sf.jsqlparser.expression.VariableAssignment; +import net.sf.jsqlparser.expression.WhenClause; +import net.sf.jsqlparser.expression.XMLSerializeExpr; +import net.sf.jsqlparser.expression.operators.arithmetic.Addition; +import net.sf.jsqlparser.expression.operators.arithmetic.BitwiseAnd; +import net.sf.jsqlparser.expression.operators.arithmetic.BitwiseLeftShift; +import net.sf.jsqlparser.expression.operators.arithmetic.BitwiseOr; +import net.sf.jsqlparser.expression.operators.arithmetic.BitwiseRightShift; +import net.sf.jsqlparser.expression.operators.arithmetic.BitwiseXor; +import net.sf.jsqlparser.expression.operators.arithmetic.Concat; +import net.sf.jsqlparser.expression.operators.arithmetic.Division; +import net.sf.jsqlparser.expression.operators.arithmetic.IntegerDivision; +import net.sf.jsqlparser.expression.operators.arithmetic.Modulo; +import net.sf.jsqlparser.expression.operators.arithmetic.Multiplication; +import net.sf.jsqlparser.expression.operators.arithmetic.Subtraction; +import net.sf.jsqlparser.expression.operators.conditional.AndExpression; +import net.sf.jsqlparser.expression.operators.conditional.OrExpression; +import net.sf.jsqlparser.expression.operators.conditional.XorExpression; +import net.sf.jsqlparser.expression.operators.relational.Between; +import net.sf.jsqlparser.expression.operators.relational.ContainedBy; +import net.sf.jsqlparser.expression.operators.relational.Contains; +import net.sf.jsqlparser.expression.operators.relational.DoubleAnd; +import net.sf.jsqlparser.expression.operators.relational.EqualsTo; +import net.sf.jsqlparser.expression.operators.relational.ExistsExpression; +import net.sf.jsqlparser.expression.operators.relational.ExpressionList; +import net.sf.jsqlparser.expression.operators.relational.FullTextSearch; +import net.sf.jsqlparser.expression.operators.relational.GeometryDistance; +import net.sf.jsqlparser.expression.operators.relational.GreaterThan; +import net.sf.jsqlparser.expression.operators.relational.GreaterThanEquals; +import net.sf.jsqlparser.expression.operators.relational.InExpression; +import net.sf.jsqlparser.expression.operators.relational.IsBooleanExpression; +import net.sf.jsqlparser.expression.operators.relational.IsDistinctExpression; +import net.sf.jsqlparser.expression.operators.relational.IsNullExpression; +import net.sf.jsqlparser.expression.operators.relational.JsonOperator; +import net.sf.jsqlparser.expression.operators.relational.LikeExpression; +import net.sf.jsqlparser.expression.operators.relational.Matches; +import net.sf.jsqlparser.expression.operators.relational.MemberOfExpression; +import net.sf.jsqlparser.expression.operators.relational.MinorThan; +import net.sf.jsqlparser.expression.operators.relational.MinorThanEquals; +import net.sf.jsqlparser.expression.operators.relational.NotEqualsTo; +import net.sf.jsqlparser.expression.operators.relational.RegExpMatchOperator; +import net.sf.jsqlparser.expression.operators.relational.SimilarToExpression; +import net.sf.jsqlparser.expression.operators.relational.TSQLLeftJoin; +import net.sf.jsqlparser.expression.operators.relational.TSQLRightJoin; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.schema.Column; +import net.sf.jsqlparser.schema.Table; +import net.sf.jsqlparser.statement.Block; +import net.sf.jsqlparser.statement.Commit; +import net.sf.jsqlparser.statement.CreateFunctionalStatement; +import net.sf.jsqlparser.statement.DeclareStatement; +import net.sf.jsqlparser.statement.DescribeStatement; +import net.sf.jsqlparser.statement.ExplainStatement; +import net.sf.jsqlparser.statement.IfElseStatement; +import net.sf.jsqlparser.statement.PurgeObjectType; +import net.sf.jsqlparser.statement.PurgeStatement; +import net.sf.jsqlparser.statement.ResetStatement; +import net.sf.jsqlparser.statement.RollbackStatement; +import net.sf.jsqlparser.statement.SavepointStatement; +import net.sf.jsqlparser.statement.SetStatement; +import net.sf.jsqlparser.statement.ShowColumnsStatement; +import net.sf.jsqlparser.statement.ShowStatement; +import net.sf.jsqlparser.statement.Statement; +import net.sf.jsqlparser.statement.StatementVisitor; +import net.sf.jsqlparser.statement.Statements; +import net.sf.jsqlparser.statement.UnsupportedStatement; +import net.sf.jsqlparser.statement.UseStatement; +import net.sf.jsqlparser.statement.alter.Alter; +import net.sf.jsqlparser.statement.alter.AlterSession; +import net.sf.jsqlparser.statement.alter.AlterSystemStatement; +import net.sf.jsqlparser.statement.alter.RenameTableStatement; +import net.sf.jsqlparser.statement.alter.sequence.AlterSequence; +import net.sf.jsqlparser.statement.analyze.Analyze; +import net.sf.jsqlparser.statement.comment.Comment; +import net.sf.jsqlparser.statement.create.index.CreateIndex; +import net.sf.jsqlparser.statement.create.schema.CreateSchema; +import net.sf.jsqlparser.statement.create.sequence.CreateSequence; +import net.sf.jsqlparser.statement.create.synonym.CreateSynonym; +import net.sf.jsqlparser.statement.create.table.CreateTable; +import net.sf.jsqlparser.statement.create.view.AlterView; +import net.sf.jsqlparser.statement.create.view.CreateView; +import net.sf.jsqlparser.statement.delete.Delete; +import net.sf.jsqlparser.statement.drop.Drop; +import net.sf.jsqlparser.statement.execute.Execute; +import net.sf.jsqlparser.statement.grant.Grant; +import net.sf.jsqlparser.statement.insert.Insert; +import net.sf.jsqlparser.statement.merge.Merge; +import net.sf.jsqlparser.statement.refresh.RefreshMaterializedViewStatement; +import net.sf.jsqlparser.statement.select.AllColumns; +import net.sf.jsqlparser.statement.select.AllTableColumns; +import net.sf.jsqlparser.statement.select.FromItemVisitor; +import net.sf.jsqlparser.statement.select.Join; +import net.sf.jsqlparser.statement.select.LateralSubSelect; +import net.sf.jsqlparser.statement.select.OrderByElement; +import net.sf.jsqlparser.statement.select.ParenthesedFromItem; +import net.sf.jsqlparser.statement.select.ParenthesedSelect; +import net.sf.jsqlparser.statement.select.PlainSelect; +import net.sf.jsqlparser.statement.select.Select; +import net.sf.jsqlparser.statement.select.SelectItem; +import net.sf.jsqlparser.statement.select.SelectItemVisitor; +import net.sf.jsqlparser.statement.select.SelectVisitor; +import net.sf.jsqlparser.statement.select.SetOperationList; +import net.sf.jsqlparser.statement.select.TableFunction; +import net.sf.jsqlparser.statement.select.TableStatement; +import net.sf.jsqlparser.statement.select.Values; +import net.sf.jsqlparser.statement.select.WithItem; +import net.sf.jsqlparser.statement.show.ShowIndexStatement; +import net.sf.jsqlparser.statement.show.ShowTablesStatement; +import net.sf.jsqlparser.statement.truncate.Truncate; +import net.sf.jsqlparser.statement.update.Update; +import net.sf.jsqlparser.statement.upsert.Upsert; + +/** + * Walks the AST, using JSqlParser's `visit()` methods. Each `visit()` method + * additionally calls a `visit____()` method (e.g., `visitColumn()`) that can be + * overriden by client classes. + */ +public class ASTWalker implements SelectVisitor, FromItemVisitor, ExpressionVisitor, + SelectItemVisitor, StatementVisitor { + + private static final String NOT_SUPPORTED_YET = "Not supported yet."; + + public ASTWalker() { + } + + /** + * Main entry point. Walk the given `expression`, calling the appropriate `visit____()` method for each element encountered. + */ + public void walk(Expression expression) { + expression.accept(this); + } + + /** + * Called for every `Column` encountered, presumably for side effects. + */ + public void visitColumn(Column column) { + } + + /** + * Called for every `Table` encountered, presumably for side effects. + */ + public void visitTable(Table table) { + } + + @Override + public void visit(Select select) { + List withItemsList = select.getWithItemsList(); + if (withItemsList != null && !withItemsList.isEmpty()) { + for (WithItem withItem : withItemsList) { + withItem.accept((SelectVisitor) this); + } + } + select.accept((SelectVisitor) this); + } + + @Override + public void visit(TranscodingFunction transcodingFunction) { + transcodingFunction.getExpression().accept(this); + } + + @Override + public void visit(TrimFunction trimFunction) { + if (trimFunction.getExpression() != null) { + trimFunction.getExpression().accept(this); + } + if (trimFunction.getFromExpression() != null) { + trimFunction.getFromExpression().accept(this); + } + } + + @Override + public void visit(RangeExpression rangeExpression) { + rangeExpression.getStartExpression().accept(this); + rangeExpression.getEndExpression().accept(this); + } + + @Override + public void visit(WithItem withItem) { + withItem.getSelect().accept((SelectVisitor) this); + } + + @Override + public void visit(ParenthesedSelect selectBody) { + List withItemsList = selectBody.getWithItemsList(); + if (withItemsList != null && !withItemsList.isEmpty()) { + for (WithItem withItem : withItemsList) { + withItem.accept((SelectVisitor) this); + } + } + selectBody.getSelect().accept((SelectVisitor) this); + } + + @Override + public void visit(PlainSelect plainSelect) { + List withItemsList = plainSelect.getWithItemsList(); + if (withItemsList != null && !withItemsList.isEmpty()) { + for (WithItem withItem : withItemsList) { + withItem.accept((SelectVisitor) this); + } + } + if (plainSelect.getSelectItems() != null) { + for (SelectItem item : plainSelect.getSelectItems()) { + item.accept(this); + } + } + + if (plainSelect.getFromItem() != null) { + plainSelect.getFromItem().accept(this); + } + + visitJoins(plainSelect.getJoins()); + if (plainSelect.getWhere() != null) { + plainSelect.getWhere().accept(this); + } + + if (plainSelect.getHaving() != null) { + plainSelect.getHaving().accept(this); + } + + if (plainSelect.getOracleHierarchical() != null) { + plainSelect.getOracleHierarchical().accept(this); + } + } + + @Override + public void visit(Table table) { + visitTable(table); + } + + @Override + public void visit(Addition addition) { + visitBinaryExpression(addition); + } + + @Override + public void visit(AndExpression andExpression) { + visitBinaryExpression(andExpression); + } + + @Override + public void visit(Between between) { + between.getLeftExpression().accept(this); + between.getBetweenExpressionStart().accept(this); + between.getBetweenExpressionEnd().accept(this); + } + + @Override + public void visit(OverlapsCondition overlapsCondition) { + overlapsCondition.getLeft().accept(this); + overlapsCondition.getRight().accept(this); + } + + @Override + public void visit(Column tableColumn) { + visitColumn(tableColumn); + if (tableColumn.getTable() != null + && tableColumn.getTable().getName() != null) { + visit(tableColumn.getTable()); + } + } + + @Override + public void visit(Division division) { + visitBinaryExpression(division); + } + + @Override + public void visit(IntegerDivision division) { + visitBinaryExpression(division); + } + + @Override + public void visit(DoubleValue doubleValue) { + + } + + @Override + public void visit(EqualsTo equalsTo) { + visitBinaryExpression(equalsTo); + } + + @Override + public void visit(Function function) { + ExpressionList exprList = function.getParameters(); + if (exprList != null) { + visit(exprList); + } + } + + @Override + public void visit(GreaterThan greaterThan) { + visitBinaryExpression(greaterThan); + } + + @Override + public void visit(GreaterThanEquals greaterThanEquals) { + visitBinaryExpression(greaterThanEquals); + } + + @Override + public void visit(InExpression inExpression) { + inExpression.getLeftExpression().accept(this); + inExpression.getRightExpression().accept(this); + } + + @Override + public void visit(FullTextSearch fullTextSearch) { + + } + + @Override + public void visit(SignedExpression signedExpression) { + signedExpression.getExpression().accept(this); + } + + @Override + public void visit(IsNullExpression isNullExpression) { + + } + + @Override + public void visit(IsBooleanExpression isBooleanExpression) { + + } + + @Override + public void visit(JdbcParameter jdbcParameter) { + + } + + @Override + public void visit(LikeExpression likeExpression) { + visitBinaryExpression(likeExpression); + } + + @Override + public void visit(ExistsExpression existsExpression) { + existsExpression.getRightExpression().accept(this); + } + + @Override + public void visit(MemberOfExpression memberOfExpression) { + memberOfExpression.getLeftExpression().accept(this); + memberOfExpression.getRightExpression().accept(this); + } + + @Override + public void visit(LongValue longValue) { + + } + + @Override + public void visit(MinorThan minorThan) { + visitBinaryExpression(minorThan); + } + + @Override + public void visit(MinorThanEquals minorThanEquals) { + visitBinaryExpression(minorThanEquals); + } + + @Override + public void visit(Multiplication multiplication) { + visitBinaryExpression(multiplication); + } + + @Override + public void visit(NotEqualsTo notEqualsTo) { + visitBinaryExpression(notEqualsTo); + } + + @Override + public void visit(DoubleAnd doubleAnd) { + visitBinaryExpression(doubleAnd); + } + + @Override + public void visit(Contains contains) { + visitBinaryExpression(contains); + } + + @Override + public void visit(ContainedBy containedBy) { + visitBinaryExpression(containedBy); + } + + @Override + public void visit(NullValue nullValue) { + + } + + @Override + public void visit(OrExpression orExpression) { + visitBinaryExpression(orExpression); + } + + @Override + public void visit(XorExpression xorExpression) { + visitBinaryExpression(xorExpression); + } + + @Override + public void visit(Parenthesis parenthesis) { + parenthesis.getExpression().accept(this); + } + + @Override + public void visit(StringValue stringValue) { + + } + + @Override + public void visit(Subtraction subtraction) { + visitBinaryExpression(subtraction); + } + + @Override + public void visit(NotExpression notExpr) { + notExpr.getExpression().accept(this); + } + + @Override + public void visit(BitwiseRightShift expr) { + visitBinaryExpression(expr); + } + + @Override + public void visit(BitwiseLeftShift expr) { + visitBinaryExpression(expr); + } + + public void visitBinaryExpression(BinaryExpression binaryExpression) { + binaryExpression.getLeftExpression().accept(this); + binaryExpression.getRightExpression().accept(this); + } + + @Override + public void visit(ExpressionList expressionList) { + for (Expression expression : expressionList) { + expression.accept(this); + } + } + + @Override + public void visit(DateValue dateValue) { + + } + + @Override + public void visit(TimestampValue timestampValue) { + + } + + @Override + public void visit(TimeValue timeValue) { + + } + + /* + * (non-Javadoc) + * + * @see net.sf.jsqlparser.expression.ExpressionVisitor#visit(net.sf.jsqlparser.expression. + * CaseExpression) + */ + @Override + public void visit(CaseExpression caseExpression) { + if (caseExpression.getSwitchExpression() != null) { + caseExpression.getSwitchExpression().accept(this); + } + if (caseExpression.getWhenClauses() != null) { + for (WhenClause when : caseExpression.getWhenClauses()) { + when.accept(this); + } + } + if (caseExpression.getElseExpression() != null) { + caseExpression.getElseExpression().accept(this); + } + } + + /* + * (non-Javadoc) + * + * @see + * net.sf.jsqlparser.expression.ExpressionVisitor#visit(net.sf.jsqlparser.expression.WhenClause) + */ + @Override + public void visit(WhenClause whenClause) { + if (whenClause.getWhenExpression() != null) { + whenClause.getWhenExpression().accept(this); + } + if (whenClause.getThenExpression() != null) { + whenClause.getThenExpression().accept(this); + } + } + + @Override + public void visit(AnyComparisonExpression anyComparisonExpression) { + anyComparisonExpression.getSelect().accept((ExpressionVisitor) this); + } + + @Override + public void visit(Concat concat) { + visitBinaryExpression(concat); + } + + @Override + public void visit(Matches matches) { + visitBinaryExpression(matches); + } + + @Override + public void visit(BitwiseAnd bitwiseAnd) { + visitBinaryExpression(bitwiseAnd); + } + + @Override + public void visit(BitwiseOr bitwiseOr) { + visitBinaryExpression(bitwiseOr); + } + + @Override + public void visit(BitwiseXor bitwiseXor) { + visitBinaryExpression(bitwiseXor); + } + + @Override + public void visit(CastExpression cast) { + cast.getLeftExpression().accept(this); + } + + @Override + public void visit(Modulo modulo) { + visitBinaryExpression(modulo); + } + + @Override + public void visit(AnalyticExpression analytic) { + if (analytic.getExpression() != null) { + analytic.getExpression().accept(this); + } + if (analytic.getDefaultValue() != null) { + analytic.getDefaultValue().accept(this); + } + if (analytic.getOffset() != null) { + analytic.getOffset().accept(this); + } + if (analytic.getKeep() != null) { + analytic.getKeep().accept(this); + } + if (analytic.getFuncOrderBy() != null) { + for (OrderByElement element : analytic.getOrderByElements()) { + element.getExpression().accept(this); + } + } + + if (analytic.getWindowElement() != null) { + analytic.getWindowElement().getRange().getStart().getExpression().accept(this); + analytic.getWindowElement().getRange().getEnd().getExpression().accept(this); + analytic.getWindowElement().getOffset().getExpression().accept(this); + } + } + + @Override + public void visit(SetOperationList list) { + List withItemsList = list.getWithItemsList(); + if (withItemsList != null && !withItemsList.isEmpty()) { + for (WithItem withItem : withItemsList) { + withItem.accept((SelectVisitor) this); + } + } + for (Select selectBody : list.getSelects()) { + selectBody.accept((SelectVisitor) this); + } + } + + @Override + public void visit(ExtractExpression eexpr) { + if (eexpr.getExpression() != null) { + eexpr.getExpression().accept(this); + } + } + + @Override + public void visit(LateralSubSelect lateralSubSelect) { + lateralSubSelect.getSelect().accept((SelectVisitor) this); + } + + @Override + public void visit(TableStatement tableStatement) { + tableStatement.getTable().accept(this); + } + + @Override + public void visit(IntervalExpression iexpr) { + if (iexpr.getExpression() != null) { + iexpr.getExpression().accept(this); + } + } + + @Override + public void visit(JdbcNamedParameter jdbcNamedParameter) { + + } + + @Override + public void visit(OracleHierarchicalExpression oexpr) { + if (oexpr.getStartExpression() != null) { + oexpr.getStartExpression().accept(this); + } + + if (oexpr.getConnectExpression() != null) { + oexpr.getConnectExpression().accept(this); + } + } + + @Override + public void visit(RegExpMatchOperator rexpr) { + visitBinaryExpression(rexpr); + } + + @Override + public void visit(JsonExpression jsonExpr) { + if (jsonExpr.getExpression() != null) { + jsonExpr.getExpression().accept(this); + } + } + + @Override + public void visit(JsonOperator jsonExpr) { + visitBinaryExpression(jsonExpr); + } + + @Override + public void visit(AllColumns allColumns) { + + } + + @Override + public void visit(AllTableColumns allTableColumns) { + + } + + @Override + public void visit(AllValue allValue) { + + } + + @Override + public void visit(IsDistinctExpression isDistinctExpression) { + visitBinaryExpression(isDistinctExpression); + } + + @Override + public void visit(SelectItem item) { + item.getExpression().accept(this); + } + + @Override + public void visit(UserVariable var) { + + } + + @Override + public void visit(NumericBind bind) { + + + } + + @Override + public void visit(KeepExpression aexpr) { + + } + + @Override + public void visit(MySQLGroupConcat groupConcat) { + + } + + @Override + public void visit(Delete delete) { + visit(delete.getTable()); + + if (delete.getUsingList() != null) { + for (Table using : delete.getUsingList()) { + visit(using); + } + } + + visitJoins(delete.getJoins()); + + if (delete.getWhere() != null) { + delete.getWhere().accept(this); + } + } + + @Override + public void visit(Update update) { + visit(update.getTable()); + if (update.getWithItemsList() != null) { + for (WithItem withItem : update.getWithItemsList()) { + withItem.accept((SelectVisitor) this); + } + } + + if (update.getStartJoins() != null) { + for (Join join : update.getStartJoins()) { + join.getRightItem().accept(this); + } + } + if (update.getExpressions() != null) { + for (Expression expression : update.getExpressions()) { + expression.accept(this); + } + } + + if (update.getFromItem() != null) { + update.getFromItem().accept(this); + } + + if (update.getJoins() != null) { + for (Join join : update.getJoins()) { + join.getRightItem().accept(this); + for (Expression expression : join.getOnExpressions()) { + expression.accept(this); + } + } + } + + if (update.getWhere() != null) { + update.getWhere().accept(this); + } + } + + @Override + public void visit(Insert insert) { + visit(insert.getTable()); + if (insert.getWithItemsList() != null) { + for (WithItem withItem : insert.getWithItemsList()) { + withItem.accept((SelectVisitor) this); + } + } + if (insert.getSelect() != null) { + visit(insert.getSelect()); + } + } + + public void visit(Analyze analyze) { + visit(analyze.getTable()); + } + + @Override + public void visit(Drop drop) { + visit(drop.getName()); + } + + @Override + public void visit(Truncate truncate) { + visit(truncate.getTable()); + } + + @Override + public void visit(CreateIndex createIndex) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(CreateSchema aThis) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(CreateTable create) { + visit(create.getTable()); + if (create.getSelect() != null) { + create.getSelect().accept((SelectVisitor) this); + } + } + + @Override + public void visit(CreateView createView) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(Alter alter) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(Statements stmts) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(Execute execute) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(SetStatement set) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(ResetStatement reset) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(ShowColumnsStatement set) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(ShowIndexStatement showIndex) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(RowConstructor rowConstructor) { + for (Expression expr : rowConstructor) { + expr.accept(this); + } + } + + @Override + public void visit(RowGetExpression rowGetExpression) { + rowGetExpression.getExpression().accept(this); + } + + @Override + public void visit(HexValue hexValue) { + + + } + + @Override + public void visit(Merge merge) { + visit(merge.getTable()); + if (merge.getWithItemsList() != null) { + for (WithItem withItem : merge.getWithItemsList()) { + withItem.accept((SelectVisitor) this); + } + } + + if (merge.getFromItem() != null) { + merge.getFromItem().accept(this); + } + } + + @Override + public void visit(OracleHint hint) { + + } + + @Override + public void visit(TableFunction tableFunction) { + visit(tableFunction.getFunction()); + } + + @Override + public void visit(AlterView alterView) { + throw new UnsupportedOperationException(NOT_SUPPORTED_YET); + } + + @Override + public void visit(RefreshMaterializedViewStatement materializedView) { + visit(materializedView.getView()); + } + + @Override + public void visit(TimeKeyExpression timeKeyExpression) { + + } + + @Override + public void visit(DateTimeLiteralExpression literal) { + + + } + + @Override + public void visit(Commit commit) { + + + } + + @Override + public void visit(Upsert upsert) { + visit(upsert.getTable()); + if (upsert.getExpressions() != null) { + upsert.getExpressions().accept(this); + } + if (upsert.getSelect() != null) { + visit(upsert.getSelect()); + } + } + + @Override + public void visit(UseStatement use) { + + } + + @Override + public void visit(ParenthesedFromItem parenthesis) { + parenthesis.getFromItem().accept(this); + // support join keyword in fromItem + visitJoins(parenthesis.getJoins()); + } + + /** + * visit join block + * + * @param parenthesis join sql block + */ + private void visitJoins(List parenthesis) { + if (parenthesis == null) { + return; + } + for (Join join : parenthesis) { + join.getFromItem().accept(this); + join.getRightItem().accept(this); + for (Expression expression : join.getOnExpressions()) { + expression.accept(this); + } + } + } + + @Override + public void visit(Block block) { + if (block.getStatements() != null) { + visit(block.getStatements()); + } + } + + @Override + public void visit(Comment comment) { + if (comment.getTable() != null) { + visit(comment.getTable()); + } + if (comment.getColumn() != null) { + Table table = comment.getColumn().getTable(); + if (table != null) { + visit(table); + } + } + } + + @Override + public void visit(Values values) { + values.getExpressions().accept(this); + } + + @Override + public void visit(DescribeStatement describe) { + describe.getTable().accept(this); + } + + @Override + public void visit(ExplainStatement explain) { + if (explain.getStatement() != null) { + explain.getStatement().accept((StatementVisitor) this); + } + } + + @Override + public void visit(NextValExpression nextVal) { + + } + + @Override + public void visit(CollateExpression col) { + col.getLeftExpression().accept(this); + } + + @Override + public void visit(ShowStatement aThis) { + + } + + @Override + public void visit(SimilarToExpression expr) { + visitBinaryExpression(expr); + } + + @Override + public void visit(DeclareStatement aThis) { + + } + + @Override + public void visit(Grant grant) { + + + } + + @Override + public void visit(ArrayExpression array) { + array.getObjExpression().accept(this); + if (array.getStartIndexExpression() != null) { + array.getIndexExpression().accept(this); + } + if (array.getStartIndexExpression() != null) { + array.getStartIndexExpression().accept(this); + } + if (array.getStopIndexExpression() != null) { + array.getStopIndexExpression().accept(this); + } + } + + @Override + public void visit(ArrayConstructor array) { + for (Expression expression : array.getExpressions()) { + expression.accept(this); + } + } + + @Override + public void visit(CreateSequence createSequence) { + throw new UnsupportedOperationException( + "Reading from a CreateSequence is not supported"); + } + + @Override + public void visit(AlterSequence alterSequence) { + throw new UnsupportedOperationException( + "Reading from an AlterSequence is not supported"); + } + + @Override + public void visit(CreateFunctionalStatement createFunctionalStatement) { + throw new UnsupportedOperationException( + "Reading from a CreateFunctionalStatement is not supported"); + } + + @Override + public void visit(ShowTablesStatement showTables) { + throw new UnsupportedOperationException( + "Reading from a ShowTablesStatement is not supported"); + } + + @Override + public void visit(TSQLLeftJoin tsqlLeftJoin) { + visitBinaryExpression(tsqlLeftJoin); + } + + @Override + public void visit(TSQLRightJoin tsqlRightJoin) { + visitBinaryExpression(tsqlRightJoin); + } + + @Override + public void visit(VariableAssignment var) { + var.getVariable().accept(this); + var.getExpression().accept(this); + } + + @Override + public void visit(XMLSerializeExpr aThis) { + + } + + @Override + public void visit(CreateSynonym createSynonym) { + throw new UnsupportedOperationException( + "Reading from a CreateSynonym is not supported"); + } + + @Override + public void visit(TimezoneExpression aThis) { + aThis.getLeftExpression().accept(this); + } + + @Override + public void visit(SavepointStatement savepointStatement) {} + + @Override + public void visit(RollbackStatement rollbackStatement) { + + } + + @Override + public void visit(AlterSession alterSession) { + + } + + @Override + public void visit(JsonAggregateFunction expression) { + Expression expr = expression.getExpression(); + if (expr != null) { + expr.accept(this); + } + + expr = expression.getFilterExpression(); + if (expr != null) { + expr.accept(this); + } + } + + @Override + public void visit(JsonFunction expression) { + for (JsonFunctionExpression expr : expression.getExpressions()) { + expr.getExpression().accept(this); + } + } + + @Override + public void visit(ConnectByRootOperator connectByRootOperator) { + connectByRootOperator.getColumn().accept(this); + } + + public void visit(IfElseStatement ifElseStatement) { + ifElseStatement.getIfStatement().accept(this); + if (ifElseStatement.getElseStatement() != null) { + ifElseStatement.getElseStatement().accept(this); + } + } + + public void visit(OracleNamedFunctionParameter oracleNamedFunctionParameter) { + oracleNamedFunctionParameter.getExpression().accept(this); + } + + @Override + public void visit(RenameTableStatement renameTableStatement) { + for (Map.Entry e : renameTableStatement.getTableNames()) { + e.getKey().accept(this); + e.getValue().accept(this); + } + } + + @Override + public void visit(PurgeStatement purgeStatement) { + if (purgeStatement.getPurgeObjectType() == PurgeObjectType.TABLE) { + ((Table) purgeStatement.getObject()).accept(this); + } + } + + @Override + public void visit(AlterSystemStatement alterSystemStatement) { + } + + @Override + public void visit(UnsupportedStatement unsupportedStatement) { + } + + @Override + public void visit(GeometryDistance geometryDistance) { + visitBinaryExpression(geometryDistance); + } +} From 13d7f4a546942ef0b123fb05f5061a0d30483bb9 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Tue, 13 Feb 2024 16:27:54 +0000 Subject: [PATCH 04/15] Almost got SqlVisitor worked out --- java/com/metabase/SqlVisitor.java | 17 ++++++++++++++++ java/com/metabase/macaw/ASTWalker.java | 26 +++++++++--------------- src/macaw/core.clj | 28 ++++++++++---------------- 3 files changed, 38 insertions(+), 33 deletions(-) create mode 100644 java/com/metabase/SqlVisitor.java diff --git a/java/com/metabase/SqlVisitor.java b/java/com/metabase/SqlVisitor.java new file mode 100644 index 0000000..6ba4970 --- /dev/null +++ b/java/com/metabase/SqlVisitor.java @@ -0,0 +1,17 @@ +package com.metabase.macaw; + +import net.sf.jsqlparser.schema.Column; +import net.sf.jsqlparser.schema.Table; + +public interface SqlVisitor { + + /** + * Called for every `Column` encountered, presumably for side effects. + */ + + public void visitColumn(Column column); + /** + * Called for every `Table` encountered, presumably for side effects. + */ + public void visitTable(Table table); +} diff --git a/java/com/metabase/macaw/ASTWalker.java b/java/com/metabase/macaw/ASTWalker.java index 63bb1f7..4aa2cc6 100644 --- a/java/com/metabase/macaw/ASTWalker.java +++ b/java/com/metabase/macaw/ASTWalker.java @@ -2,6 +2,8 @@ // Borrows substantially from JSqlParser's TablesNamesFinder +import com.metabase.macaw.SqlVisitor; + import java.util.List; import java.util.Map; @@ -176,29 +178,21 @@ public class ASTWalker implements SelectVisitor, FromItemVisitor, ExpressionVisi SelectItemVisitor, StatementVisitor { private static final String NOT_SUPPORTED_YET = "Not supported yet."; + private SqlVisitor sqlVisitor; - public ASTWalker() { + public ASTWalker(SqlVisitor sqlVisitor) { + this.sqlVisitor = sqlVisitor; } /** - * Main entry point. Walk the given `expression`, calling the appropriate `visit____()` method for each element encountered. + * Main entry point. Walk the given `expression`, calling the appropriate + * `visit____()` method from the [[SqlVisitor]] for each element + * encountered. */ public void walk(Expression expression) { expression.accept(this); } - /** - * Called for every `Column` encountered, presumably for side effects. - */ - public void visitColumn(Column column) { - } - - /** - * Called for every `Table` encountered, presumably for side effects. - */ - public void visitTable(Table table) { - } - @Override public void visit(Select select) { List withItemsList = select.getWithItemsList(); @@ -281,7 +275,7 @@ public void visit(PlainSelect plainSelect) { @Override public void visit(Table table) { - visitTable(table); + this.sqlVisitor.visitTable(table); } @Override @@ -309,7 +303,7 @@ public void visit(OverlapsCondition overlapsCondition) { @Override public void visit(Column tableColumn) { - visitColumn(tableColumn); + this.sqlVisitor.visitColumn(tableColumn); if (tableColumn.getTable() != null && tableColumn.getTable().getName() != null) { visit(tableColumn.getTable()); diff --git a/src/macaw/core.clj b/src/macaw/core.clj index 4717e71..f5085da 100644 --- a/src/macaw/core.clj +++ b/src/macaw/core.clj @@ -1,5 +1,8 @@ (ns macaw.core (:import + (com.metabase.macaw + ASTWalker + SqlVisitor) (net.sf.jsqlparser Model) (net.sf.jsqlparser.parser @@ -38,27 +41,18 @@ (println "CONJing!") (swap! known-column-names conj (.getColumnName column)))) + (defn query->columns "TODO: implement!" [^Statement parsed-query] - (println "query->columns") (let [column-names (atom []) - column-finder (proxy - [TablesNamesFinder] - [] - (visit [visitable] - (println "visiting") - (println {:visitable visitable - :column-names @column-names}) - (let [^TablesNamesFinder this this] - (conj-column! visitable column-names) - (proxy-super visit visitable))))] -#_ (.init column-finder false) -#_ (.accept parsed-query column-finder) - - (.getTables column-finder parsed-query) - #_ @column-names) -) + column-finder (reify + SqlVisitor + (visitColumn (_this ^Column column) + (swap! column-names conj (.getColumnName column))) + (visitTable (_this _table)))] + (.walk (ASTWalker. column-finder) parsed-query) + @column-names)) (defn parsed-query From 9f01e145f0fd6f81dd875cef7bcec3cc3a8069da Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Thu, 15 Feb 2024 11:07:09 +0000 Subject: [PATCH 05/15] Add SqlVisitor and ASTWalker; use for query->columns --- README.md | 14 +++++-- deps.edn | 2 +- java/com/metabase/SqlVisitor.java | 33 +++++++++++++++- java/com/metabase/macaw/ASTWalker.java | 5 +-- src/macaw/core.clj | 54 ++++---------------------- test/macaw/core_test.clj | 7 ++++ 6 files changed, 59 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index bca7784..ed83d2a 100644 --- a/README.md +++ b/README.md @@ -23,11 +23,17 @@ clj -T:build jar This will create a JAR in the `target` directory. -The build process is slightly complicated since Macaw mixes Clojure and Java -files. If you're working on Macaw itself and make changes to a Java file, you -must: +## Working with the Java files -1. Rebuild +To compile the Java files, use + +``` +clj -T:build compile +``` + +If you're working on Macaw and make changes to a Java file, you must: + +1. Recompile 2. Restart your Clojure REPL for the changes to take effect. diff --git a/deps.edn b/deps.edn index 01557ac..ec44143 100644 --- a/deps.edn +++ b/deps.edn @@ -1,5 +1,5 @@ {:paths - ["src" "java" "resources"] + ["src" "java" "resources" "target/classes"] :deps {com.github.jsqlparser/jsqlparser {:mvn/version "4.8"}} ; The actual SQL Parser to wrap! diff --git a/java/com/metabase/SqlVisitor.java b/java/com/metabase/SqlVisitor.java index 6ba4970..b080a8b 100644 --- a/java/com/metabase/SqlVisitor.java +++ b/java/com/metabase/SqlVisitor.java @@ -3,13 +3,44 @@ import net.sf.jsqlparser.schema.Column; import net.sf.jsqlparser.schema.Table; +/** + * Clojure is not good at working with Java Visitors. They require overriding various overloaded + * methods and, in the case of walking a tree (exactly what we want to do here) can be counted on to call `visit()` + * recursively. + * + * Clojure's two main ways of dealing with this are `reify`, which does not permit overloading, and `proxy`, which does. + * However, creating a proxy object creates a completely new object that does not inherit behavior defined in the parent + * class. Therefore, if you have code like this: + * + * + (proxy + [TablesNamesFinder] + [] + (visit [visitable] + (if (instance? Column visitable) + (swap! columns conj (.getColumnName ^Column visitable)) + (let [^StatementVisitor this this] + (proxy-super visit visitable))))) + + + * the call to `proxy-super` does not call `TablesNamesFinder.visit()` with the non-`Column` `visitable`; that + * definition has been lost. + * + *
+ * + * Therefore, this interface was created to provide a more convenient escape hatch for Clojure. It removes the + * overloading requirement for the conventional visitor pattern, instead providing differently-named methods for each + * type. This lets Clojure code use `reify` to implement each method with the necessary behavior. The recursive + * tree-walking is handled by a different class, which calls the methods defined here along the way. Think of them as + * hooks for Clojure-land that don't affect the main behavior of the visitor. + */ public interface SqlVisitor { /** * Called for every `Column` encountered, presumably for side effects. */ - public void visitColumn(Column column); + /** * Called for every `Table` encountered, presumably for side effects. */ diff --git a/java/com/metabase/macaw/ASTWalker.java b/java/com/metabase/macaw/ASTWalker.java index 4aa2cc6..ba7219d 100644 --- a/java/com/metabase/macaw/ASTWalker.java +++ b/java/com/metabase/macaw/ASTWalker.java @@ -170,9 +170,8 @@ import net.sf.jsqlparser.statement.upsert.Upsert; /** - * Walks the AST, using JSqlParser's `visit()` methods. Each `visit()` method - * additionally calls a `visit____()` method (e.g., `visitColumn()`) that can be - * overriden by client classes. + * Walks the AST, using JSqlParser's `visit()` methods. Each `visit()` method additionally calls a `visit____()` method + * (e.g., `visitColumn()`) as defined in the [[SqlVisitor]] interface that can be overriden by client classes. */ public class ASTWalker implements SelectVisitor, FromItemVisitor, ExpressionVisitor, SelectItemVisitor, StatementVisitor { diff --git a/src/macaw/core.clj b/src/macaw/core.clj index f5085da..77d8878 100644 --- a/src/macaw/core.clj +++ b/src/macaw/core.clj @@ -10,61 +10,37 @@ (net.sf.jsqlparser.schema Column) (net.sf.jsqlparser.statement - Statement - StatementVisitor) - (net.sf.jsqlparser.statement.update - Update) + Statement) (net.sf.jsqlparser.util TablesNamesFinder))) #_(set! *warn-on-reflection* true) (defn query->tables - "Given a parsed query (i.e., a subclass of `Statement`) return a list of fully-qualified table names found within it. + "Given a parsed query (i.e., a [subclass of] `Statement`) return a list of fully-qualified table names found within it. Note: 'fully-qualified' means 'as found in the query'; it doesn't extrapolate schema names from other data sources." [^Statement parsed-query] (let [table-finder (TablesNamesFinder.)] (.getTableList table-finder parsed-query))) -(defprotocol ConjColumn - (conj-column! [x known-columns])) - -(extend-protocol ConjColumn - Model - (conj-column! [_ _known-column-names] - (println "nothing to add") - nil) - - Column - (conj-column! [^Column column known-column-names] - (println "CONJing!") - (swap! known-column-names conj (.getColumnName column)))) - - (defn query->columns - "TODO: implement!" + "Given a parsed query (i.e., a [subclass of] `Statement`) return a list of the column names found within it.)" [^Statement parsed-query] (let [column-names (atom []) column-finder (reify SqlVisitor - (visitColumn (_this ^Column column) + (^void visitColumn [_this ^Column column] (swap! column-names conj (.getColumnName column))) - (visitTable (_this _table)))] + (visitTable [_this _table]))] (.walk (ASTWalker. column-finder) parsed-query) @column-names)) - (defn parsed-query "Main entry point: takes a string query and returns a `Statement` object that can be handled by the other functions." [^String query] (CCJSqlParserUtil/parse query)) -(-> "select foo, bar from baz;" - parsed-query - query->columns) - - (defn resolve-columns "TODO: Make this use metadata we know about. TODO: If nil is a column (from a select *) then no need for the rest of the entries @@ -80,23 +56,7 @@ (defn lineage "Returns a sequence of the columns used in / referenced by the query" [query] - (let [parsed (parsed-query query) - tables (query->tables parsed) + (let [parsed (parsed-query query) + tables (query->tables parsed) columns (query->columns parsed)] (resolve-columns tables columns))) - - - - - -(comment - - - - @(u/prog1 (atom []) - (conj-column! 1 <>) - (conj-column! 2.0 <>) - (conj-column! (Integer. 8) <>)) - - - ) diff --git a/test/macaw/core_test.clj b/test/macaw/core_test.clj index de0d14c..f0addb6 100644 --- a/test/macaw/core_test.clj +++ b/test/macaw/core_test.clj @@ -18,6 +18,13 @@ (is (= ["core_user"] (tables "select * from (select distinct email from core_user) q;"))))) +(def columns (comp set m/query->columns m/parsed-query)) + +(deftest ^:parallel query->columns-test + (testing "Simple queries" + (is (= #{"foo" "bar" "id" "quux_id"} + (columns "select foo, bar from baz inner join quux on quux.id = baz.quux_id"))))) + (deftest ^:parallel resolve-columns-test (let [cols ["name" "id" "email"]] (is (= {"core_user" cols From 27fc0039c029e482aea5869cce35f780e7ba1a9b Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Thu, 15 Feb 2024 11:14:09 +0000 Subject: [PATCH 06/15] Cleanup --- src/macaw/core.clj | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/macaw/core.clj b/src/macaw/core.clj index 77d8878..349cef3 100644 --- a/src/macaw/core.clj +++ b/src/macaw/core.clj @@ -3,8 +3,6 @@ (com.metabase.macaw ASTWalker SqlVisitor) - (net.sf.jsqlparser - Model) (net.sf.jsqlparser.parser CCJSqlParserUtil) (net.sf.jsqlparser.schema @@ -14,7 +12,7 @@ (net.sf.jsqlparser.util TablesNamesFinder))) -#_(set! *warn-on-reflection* true) +(set! *warn-on-reflection* true) (defn query->tables "Given a parsed query (i.e., a [subclass of] `Statement`) return a list of fully-qualified table names found within it. From 981b4766f0e1317c0eae0fbca1b5417e4bbc4f26 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Thu, 15 Feb 2024 11:59:37 +0000 Subject: [PATCH 07/15] Build before testing on CI --- .github/workflows/tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d34a965..5a48e64 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -46,6 +46,8 @@ jobs: restore-keys: | v1-${{ hashFiles('./deps.edn') }}- v1- + - run: clojure -T:build compile + name: Compile Java files - run: clojure -X:dev:test name: Run tests env: From 9ea7361a089182e11f13a08985bfa74ae0118a9f Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Thu, 15 Feb 2024 12:00:18 +0000 Subject: [PATCH 08/15] Build before checking on CI --- .github/workflows/tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5a48e64..108e090 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -104,6 +104,8 @@ jobs: restore-keys: | v1-${{ hashFiles('./deps.edn') }}- v1- + - run: clojure -T:build compile + name: Compile Java files - run: clojure -M:check name: Check namespaces From 7a3164a253c152e2b2cea43e7d72801e22c1e291 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Thu, 15 Feb 2024 16:01:32 +0000 Subject: [PATCH 09/15] Put Java file in the right spot :open_mouth: --- java/com/metabase/{ => macaw}/SqlVisitor.java | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename java/com/metabase/{ => macaw}/SqlVisitor.java (100%) diff --git a/java/com/metabase/SqlVisitor.java b/java/com/metabase/macaw/SqlVisitor.java similarity index 100% rename from java/com/metabase/SqlVisitor.java rename to java/com/metabase/macaw/SqlVisitor.java From 1efb3cccd1515c1eac4706cc14d932d54085b820 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Mon, 19 Feb 2024 17:05:57 +0000 Subject: [PATCH 10/15] Refactor to use a Clojure map + callbacks --- java/com/metabase/macaw/ASTWalker.java | 70 +++++++++++++++++++++---- java/com/metabase/macaw/SqlVisitor.java | 48 ----------------- src/macaw/core.clj | 46 +++++++--------- test/macaw/core_test.clj | 12 ++--- 4 files changed, 84 insertions(+), 92 deletions(-) delete mode 100644 java/com/metabase/macaw/SqlVisitor.java diff --git a/java/com/metabase/macaw/ASTWalker.java b/java/com/metabase/macaw/ASTWalker.java index ba7219d..160486c 100644 --- a/java/com/metabase/macaw/ASTWalker.java +++ b/java/com/metabase/macaw/ASTWalker.java @@ -2,8 +2,10 @@ // Borrows substantially from JSqlParser's TablesNamesFinder -import com.metabase.macaw.SqlVisitor; +import clojure.lang.IFn; +import clojure.lang.Keyword; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -170,23 +172,69 @@ import net.sf.jsqlparser.statement.upsert.Upsert; /** - * Walks the AST, using JSqlParser's `visit()` methods. Each `visit()` method additionally calls a `visit____()` method - * (e.g., `visitColumn()`) as defined in the [[SqlVisitor]] interface that can be overriden by client classes. + * Walks the AST, using JSqlParser's `visit()` methods. Each `visit()` method additionally calls an applicable callback method provided in the `callbacks` map. + * Supported callbacks have a corresponding key string (see below). + * + * Why this class? Why the callbacks? + * + * Clojure is not good at working with Java Visitors. They require overriding various overloaded + * methods and, in the case of walking a tree (exactly what we want to do here) we of course need to call `visit()` + * recursively. + * + * Clojure's two main ways of dealing with this are `reify`, which does not permit type-based overloading, and `proxy`, + * which does. However, creating a proxy object creates a completely new object that does not inherit behavior defined + * in the parent class. Therefore, if you have code like this: + * + * + (proxy + [TablesNamesFinder] + [] + (visit [visitable] + (if (instance? Column visitable) + (swap! columns conj (.getColumnName ^Column visitable)) + (let [^StatementVisitor this this] + (proxy-super visit visitable))))) + + + * the call to `proxy-super` does not call the original `TablesNamesFinder.visit()`; it calls `visit()` on our + * proxied instance. Since the tree-walking semantics are defined in the original method, this behavior does not work + * for us. + * + *
+ * + * Therefore, this class provides a more convenient escape hatch for Clojure. It removes the overloading requirement for + * the conventional visitor pattern, instead providing the `callbacks` map This lets Clojure code use a normal Clojure + * map and functions to implement the necessary behavior; no `reify` necesary. */ public class ASTWalker implements SelectVisitor, FromItemVisitor, ExpressionVisitor, SelectItemVisitor, StatementVisitor { + public static final String COLUMN_STR = "column"; + public static final String TABLE_STR = "table"; + public static final String[] SUPPORTED_CALLBACK_KEYS = new String[] { COLUMN_STR, TABLE_STR }; + private static final String NOT_SUPPORTED_YET = "Not supported yet."; - private SqlVisitor sqlVisitor; + private Map callbacks; - public ASTWalker(SqlVisitor sqlVisitor) { - this.sqlVisitor = sqlVisitor; + /** + * Construct a new walker with the given `callbacks`. The `callbacks` should be a (Clojure) map like so: + * + * 

+     *   (ASTWalker. {:column (fn [col] (do-something-with-the-found col))
+     *                :table  (fn [table] (...))})
+     *
+ * + * The appropriate callback fn will be invoked for every matching element found. The list of supported keys can be found in [[SUPPORTED_CALLBACK_KEYS]]. + */ + public ASTWalker(Map callbacksWithKeywordKeys) { + this.callbacks = new HashMap(SUPPORTED_CALLBACK_KEYS.length); + for(Map.Entry entry : callbacksWithKeywordKeys.entrySet()) { + this.callbacks.put(entry.getKey().getName(), entry.getValue()); + } } /** - * Main entry point. Walk the given `expression`, calling the appropriate - * `visit____()` method from the [[SqlVisitor]] for each element - * encountered. + * Main entry point. Walk the given `expression`, invoking the callbacks as appropriate. */ public void walk(Expression expression) { expression.accept(this); @@ -274,7 +322,7 @@ public void visit(PlainSelect plainSelect) { @Override public void visit(Table table) { - this.sqlVisitor.visitTable(table); + this.callbacks.get(TABLE_STR).invoke(table); } @Override @@ -302,7 +350,7 @@ public void visit(OverlapsCondition overlapsCondition) { @Override public void visit(Column tableColumn) { - this.sqlVisitor.visitColumn(tableColumn); + this.callbacks.get(COLUMN_STR).invoke(tableColumn); if (tableColumn.getTable() != null && tableColumn.getTable().getName() != null) { visit(tableColumn.getTable()); diff --git a/java/com/metabase/macaw/SqlVisitor.java b/java/com/metabase/macaw/SqlVisitor.java deleted file mode 100644 index b080a8b..0000000 --- a/java/com/metabase/macaw/SqlVisitor.java +++ /dev/null @@ -1,48 +0,0 @@ -package com.metabase.macaw; - -import net.sf.jsqlparser.schema.Column; -import net.sf.jsqlparser.schema.Table; - -/** - * Clojure is not good at working with Java Visitors. They require overriding various overloaded - * methods and, in the case of walking a tree (exactly what we want to do here) can be counted on to call `visit()` - * recursively. - * - * Clojure's two main ways of dealing with this are `reify`, which does not permit overloading, and `proxy`, which does. - * However, creating a proxy object creates a completely new object that does not inherit behavior defined in the parent - * class. Therefore, if you have code like this: - * - * - (proxy - [TablesNamesFinder] - [] - (visit [visitable] - (if (instance? Column visitable) - (swap! columns conj (.getColumnName ^Column visitable)) - (let [^StatementVisitor this this] - (proxy-super visit visitable))))) - - - * the call to `proxy-super` does not call `TablesNamesFinder.visit()` with the non-`Column` `visitable`; that - * definition has been lost. - * - *
- * - * Therefore, this interface was created to provide a more convenient escape hatch for Clojure. It removes the - * overloading requirement for the conventional visitor pattern, instead providing differently-named methods for each - * type. This lets Clojure code use `reify` to implement each method with the necessary behavior. The recursive - * tree-walking is handled by a different class, which calls the methods defined here along the way. Think of them as - * hooks for Clojure-land that don't affect the main behavior of the visitor. - */ -public interface SqlVisitor { - - /** - * Called for every `Column` encountered, presumably for side effects. - */ - public void visitColumn(Column column); - - /** - * Called for every `Table` encountered, presumably for side effects. - */ - public void visitTable(Table table); -} diff --git a/src/macaw/core.clj b/src/macaw/core.clj index 349cef3..2eb00d5 100644 --- a/src/macaw/core.clj +++ b/src/macaw/core.clj @@ -1,38 +1,31 @@ (ns macaw.core (:import (com.metabase.macaw - ASTWalker - SqlVisitor) + ASTWalker) (net.sf.jsqlparser.parser CCJSqlParserUtil) (net.sf.jsqlparser.schema - Column) + Column + Table) (net.sf.jsqlparser.statement - Statement) - (net.sf.jsqlparser.util - TablesNamesFinder))) + Statement))) (set! *warn-on-reflection* true) -(defn query->tables - "Given a parsed query (i.e., a [subclass of] `Statement`) return a list of fully-qualified table names found within it. +(defn query->components + "Given a parsed query (i.e., a [subclass of] `Statement`) return a map with the `:tables` and `:columns` found within it. - Note: 'fully-qualified' means 'as found in the query'; it doesn't extrapolate schema names from other data sources." + (Specifically, it returns their fully-qualified names as strings, where 'fully-qualified' means 'as found in the query'.)" [^Statement parsed-query] - (let [table-finder (TablesNamesFinder.)] - (.getTableList table-finder parsed-query))) - -(defn query->columns - "Given a parsed query (i.e., a [subclass of] `Statement`) return a list of the column names found within it.)" - [^Statement parsed-query] - (let [column-names (atom []) - column-finder (reify - SqlVisitor - (^void visitColumn [_this ^Column column] - (swap! column-names conj (.getColumnName column))) - (visitTable [_this _table]))] - (.walk (ASTWalker. column-finder) parsed-query) - @column-names)) + (let [column-names (atom #{}) + table-names (atom #{}) + ast-walker (ASTWalker. {:column (fn [^Column column] + (swap! column-names conj (.getColumnName column))) + :table (fn [^Table table] + (swap! table-names conj (.getFullyQualifiedName table)))})] + (.walk ast-walker parsed-query) + {:columns @column-names + :tables @table-names})) (defn parsed-query "Main entry point: takes a string query and returns a `Statement` object that can be handled by the other functions." @@ -46,7 +39,7 @@ [tables columns] (let [cartesian-product (for [table tables column columns] - {:table table + {:table table :column column})] (update-vals (group-by :table cartesian-product) #(merge-with concat (map :column %))))) @@ -54,7 +47,6 @@ (defn lineage "Returns a sequence of the columns used in / referenced by the query" [query] - (let [parsed (parsed-query query) - tables (query->tables parsed) - columns (query->columns parsed)] + (let [parsed (parsed-query query) + {:keys [columns tables]} (query->components parsed)] (resolve-columns tables columns))) diff --git a/test/macaw/core_test.clj b/test/macaw/core_test.clj index f0addb6..284d0f1 100644 --- a/test/macaw/core_test.clj +++ b/test/macaw/core_test.clj @@ -3,22 +3,22 @@ [clojure.test :refer :all] [macaw.core :as m])) -(def tables (comp m/query->tables m/parsed-query)) +(def tables (comp :tables m/query->components m/parsed-query)) (deftest ^:parallel query->tables-test (testing "Simple queries" - (is (= ["core_user"] + (is (= #{"core_user"} (tables "select * from core_user;"))) - (is (= ["core_user"] + (is (= #{"core_user"} (tables "select id, email from core_user;")))) (testing "With a schema (Postgres)" ;; TODO: only run this against supported DBs - (is (= ["the_schema_name.core_user"] + (is (= #{"the_schema_name.core_user"} (tables "select * from the_schema_name.core_user;")))) (testing "Sub-selects" - (is (= ["core_user"] + (is (= #{"core_user"} (tables "select * from (select distinct email from core_user) q;"))))) -(def columns (comp set m/query->columns m/parsed-query)) +(def columns (comp :columns m/query->components m/parsed-query)) (deftest ^:parallel query->columns-test (testing "Simple queries" From c2a178c7751a565a96a00e8077f9aa89be092066 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Mon, 19 Feb 2024 17:07:47 +0000 Subject: [PATCH 11/15] Add some basic build scripts --- bin/build-jar | 25 +++++++++++++++++++++++++ bin/java-compile | 25 +++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100755 bin/build-jar create mode 100755 bin/java-compile diff --git a/bin/build-jar b/bin/build-jar new file mode 100755 index 0000000..978f7c8 --- /dev/null +++ b/bin/build-jar @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +set -o errexit +set -o nounset +set -o pipefail +if [[ -n "${TRACE-}" ]]; then + set -o xtrace +fi + +if [ $# -ge 1 ] && [[ "$1" =~ ^-*h(elp)?$ ]]; then + echo 'Usage: ./bin/build-jar + + Build a JAR + +' + exit +fi + +# Ensure we're in the project root +cd "$(dirname "$0")"/.. + +main() { + clj -T:build jar +} + +main "$@" diff --git a/bin/java-compile b/bin/java-compile new file mode 100755 index 0000000..5483965 --- /dev/null +++ b/bin/java-compile @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +set -o errexit +set -o nounset +set -o pipefail +if [[ -n "${TRACE-}" ]]; then + set -o xtrace +fi + +if [ $# -ge 1 ] && [[ "$1" =~ ^-*h(elp)?$ ]]; then + echo 'Usage: ./bin/java-compile + + Compiles all the Java files used in the project so that they can be used in local Clojure development. To build a JAR, use ./build-jar instead. + +' + exit +fi + +# Ensure we're in the project root +cd "$(dirname "$0")"/.. + +main() { + clj -T:build compile +} + +main "$@" From e5dfbea194db939c576dda3b9ae6eba6cfa30bf1 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Mon, 19 Feb 2024 17:08:35 +0000 Subject: [PATCH 12/15] Update docs --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ed83d2a..b26b62f 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ namesake, it's intelligent, can be taught to speak SQL, and has many colors To build a local JAR, use ``` -clj -T:build jar +./bin/build-jar ``` This will create a JAR in the `target` directory. @@ -28,7 +28,7 @@ This will create a JAR in the `target` directory. To compile the Java files, use ``` -clj -T:build compile +./bin/compile-java ``` If you're working on Macaw and make changes to a Java file, you must: From 7e7cbebdd64d2413dc4390a61044b2daa437e075 Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Tue, 20 Feb 2024 10:03:09 +0000 Subject: [PATCH 13/15] atom->transient --- src/macaw/core.clj | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/macaw/core.clj b/src/macaw/core.clj index 2eb00d5..6479cc3 100644 --- a/src/macaw/core.clj +++ b/src/macaw/core.clj @@ -17,15 +17,15 @@ (Specifically, it returns their fully-qualified names as strings, where 'fully-qualified' means 'as found in the query'.)" [^Statement parsed-query] - (let [column-names (atom #{}) - table-names (atom #{}) + (let [column-names (transient #{}) + table-names (transient #{}) ast-walker (ASTWalker. {:column (fn [^Column column] - (swap! column-names conj (.getColumnName column))) + (conj! column-names (.getColumnName column))) :table (fn [^Table table] - (swap! table-names conj (.getFullyQualifiedName table)))})] + (conj! table-names (.getFullyQualifiedName table)))})] (.walk ast-walker parsed-query) - {:columns @column-names - :tables @table-names})) + {:columns (persistent! column-names) + :tables (persistent! table-names)})) (defn parsed-query "Main entry point: takes a string query and returns a `Statement` object that can be handled by the other functions." From 18c5e0d3145a2986e7dbbea532fc7d38546b4b5f Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Tue, 20 Feb 2024 15:32:03 +0000 Subject: [PATCH 14/15] Make ASTWalker more robust/foolproof --- java/com/metabase/macaw/ASTWalker.java | 30 ++++++++++++++++++++------ 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/java/com/metabase/macaw/ASTWalker.java b/java/com/metabase/macaw/ASTWalker.java index 160486c..0e7991c 100644 --- a/java/com/metabase/macaw/ASTWalker.java +++ b/java/com/metabase/macaw/ASTWalker.java @@ -8,6 +8,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import net.sf.jsqlparser.JSQLParserException; import net.sf.jsqlparser.expression.AllValue; @@ -184,8 +185,8 @@ * Clojure's two main ways of dealing with this are `reify`, which does not permit type-based overloading, and `proxy`, * which does. However, creating a proxy object creates a completely new object that does not inherit behavior defined * in the parent class. Therefore, if you have code like this: - * - * + + (proxy [TablesNamesFinder] [] @@ -211,7 +212,7 @@ public class ASTWalker implements SelectVisitor, FromItemVisitor, ExpressionVisi public static final String COLUMN_STR = "column"; public static final String TABLE_STR = "table"; - public static final String[] SUPPORTED_CALLBACK_KEYS = new String[] { COLUMN_STR, TABLE_STR }; + public static final Set SUPPORTED_CALLBACK_KEYS = Set.of(COLUMN_STR, TABLE_STR); private static final String NOT_SUPPORTED_YET = "Not supported yet."; private Map callbacks; @@ -225,11 +226,26 @@ public class ASTWalker implements SelectVisitor, FromItemVisitor, ExpressionVisi * * * The appropriate callback fn will be invoked for every matching element found. The list of supported keys can be found in [[SUPPORTED_CALLBACK_KEYS]]. + * + * Silently rejects invalid keys. */ public ASTWalker(Map callbacksWithKeywordKeys) { - this.callbacks = new HashMap(SUPPORTED_CALLBACK_KEYS.length); + this.callbacks = new HashMap(SUPPORTED_CALLBACK_KEYS.size()); for(Map.Entry entry : callbacksWithKeywordKeys.entrySet()) { - this.callbacks.put(entry.getKey().getName(), entry.getValue()); + String keyName = entry.getKey().getName(); + if (SUPPORTED_CALLBACK_KEYS.contains(keyName)) { + this.callbacks.put(keyName, entry.getValue()); + } + } + } + + /** + * Safely invoke the given callback by name. + */ + public void invokeCallback(String callbackName, Object visitedItem) { + IFn callback = this.callbacks.get(callbackName); + if (callback != null) { + callback.invoke(visitedItem); } } @@ -322,7 +338,7 @@ public void visit(PlainSelect plainSelect) { @Override public void visit(Table table) { - this.callbacks.get(TABLE_STR).invoke(table); + invokeCallback(TABLE_STR, table); } @Override @@ -350,7 +366,7 @@ public void visit(OverlapsCondition overlapsCondition) { @Override public void visit(Column tableColumn) { - this.callbacks.get(COLUMN_STR).invoke(tableColumn); + invokeCallback(COLUMN_STR, tableColumn); if (tableColumn.getTable() != null && tableColumn.getTable().getName() != null) { visit(tableColumn.getTable()); From 9af463e7df565514739b97b6899962ac9a9e5b8e Mon Sep 17 00:00:00 2001 From: Tim Macdonald Date: Tue, 20 Feb 2024 15:32:11 +0000 Subject: [PATCH 15/15] Back to atoms --- java/com/metabase/macaw/ASTWalker.java | 6 +++--- src/macaw/core.clj | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/java/com/metabase/macaw/ASTWalker.java b/java/com/metabase/macaw/ASTWalker.java index 0e7991c..68d3dc8 100644 --- a/java/com/metabase/macaw/ASTWalker.java +++ b/java/com/metabase/macaw/ASTWalker.java @@ -173,12 +173,12 @@ import net.sf.jsqlparser.statement.upsert.Upsert; /** - * Walks the AST, using JSqlParser's `visit()` methods. Each `visit()` method additionally calls an applicable callback method provided in the `callbacks` map. - * Supported callbacks have a corresponding key string (see below). + * Walks the AST, using JSqlParser's `visit()` methods. Each `visit()` method additionally calls an applicable callback + * method provided in the `callbacks` map. Supported callbacks have a corresponding key string (see below). * * Why this class? Why the callbacks? * - * Clojure is not good at working with Java Visitors. They require overriding various overloaded + * Clojure is not good at working with Java visitors. They require overriding various overloaded * methods and, in the case of walking a tree (exactly what we want to do here) we of course need to call `visit()` * recursively. * diff --git a/src/macaw/core.clj b/src/macaw/core.clj index 6479cc3..89037b1 100644 --- a/src/macaw/core.clj +++ b/src/macaw/core.clj @@ -15,17 +15,17 @@ (defn query->components "Given a parsed query (i.e., a [subclass of] `Statement`) return a map with the `:tables` and `:columns` found within it. - (Specifically, it returns their fully-qualified names as strings, where 'fully-qualified' means 'as found in the query'.)" + (Specifically, it returns their fully-qualified names as strings, where 'fully-qualified' means 'as referred to in the query'; this function doesn't do additional inference work to find out a table's schema.)" [^Statement parsed-query] - (let [column-names (transient #{}) - table-names (transient #{}) + (let [column-names (atom #{}) + table-names (atom #{}) ast-walker (ASTWalker. {:column (fn [^Column column] - (conj! column-names (.getColumnName column))) + (swap! column-names conj (.getColumnName column))) :table (fn [^Table table] - (conj! table-names (.getFullyQualifiedName table)))})] + (swap! table-names conj (.getFullyQualifiedName table)))})] (.walk ast-walker parsed-query) - {:columns (persistent! column-names) - :tables (persistent! table-names)})) + {:columns @column-names + :tables @table-names})) (defn parsed-query "Main entry point: takes a string query and returns a `Statement` object that can be handled by the other functions."