Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some sort of support for CREATE INDEX #348

Closed
seancorfield opened this issue Aug 31, 2021 · 7 comments · Fixed by #517
Closed

Add some sort of support for CREATE INDEX #348

seancorfield opened this issue Aug 31, 2021 · 7 comments · Fixed by #517
Assignees

Comments

@seancorfield
Copy link
Owner

There's no standard for this aspect of DDL it seems, but here's:

They're different (of course) but have some common elements. It's not clear what HoneySQL syntax should be for this because there are some words that come between CREATE and INDEX, some that come between INDEX and the name (which is optional in PG but not in MS! Argh!) and WITH/WHERE are in different orders between PG and MS too. Nasty.

@seancorfield
Copy link
Owner Author

MySQL https://dev.mysql.com/doc/refman/5.7/en/create-index.html -- yet more differences.

@seancorfield seancorfield self-assigned this Oct 13, 2021
@seancorfield seancorfield added the needs analysis I need to think about this! label Dec 4, 2021
@seancorfield
Copy link
Owner Author

Since there is already ALTER TABLE and ADD INDEX I'm going to close this out as "won't fix" due to the syntax variability (just as I closed out a CREATE FUNCTION ticket).

@dancek
Copy link
Contributor

dancek commented Dec 5, 2023

PostgreSQL has CREATE INDEX IF NOT EXISTS which allows creating indices idempotently. It would be quite useful for schema creation (you can already construct databases idempotently with :if-not-exists for :create-table and :alter-table :add-column). I do realize supporting a syntax that's very different in different databases and somewhat overlaps with :add-index is a pain.

But if I were to implement Postgres-specific opt-in support similarly to how honey.sql.pg-ops does for operators, would you consider merging that?

@seancorfield
Copy link
Owner Author

Sure, if you want to write up a PR for a PG-specific :create-index, with tests and doc updates, I'll take a look at it.

I'd prefer something generic enough to be part of the HoneySQL core and to be flexible enough for MySQL and SQL Server but I'm not likely to use it so that's not a requirement (I don't use HoneySQL for any DDL because it's just too irregular).

@dancek
Copy link
Contributor

dancek commented Dec 7, 2023

For my use case, this would be enough:

(ns honey.sql.pg-ddl
  (:require [honey.sql :as sql]))

(sql/register-clause! :create-index
                      (fn [_ x] (sql/format-create :create :index x nil))
                      :raw)

(sql/register-clause! :create-unique-index
                      (fn [_ x] (sql/format-create :create :unique-index x nil))
                      :raw)

(sql/register-clause! :on
                      (fn [_ [table column]]
                        [(str "ON " (sql/format-entity table) 
                              " (" (sql/format-entity column) ")")])
                      nil)

which works like this:

(ns honey.sql.pg-ddl-test
  (:require [clojure.test :refer [deftest is testing]]
            [honey.sql :as sql]
            [honey.sql.pg-ddl]))

(deftest pg-ddl-tests
  (testing "create index"
    (is (= ["CREATE INDEX my_column_idx ON my_table (my_column)"]
           (sql/format {:create-index :my-column-idx
                        :on [:my-table :my-column]})))
    (testing "if not exists"
      (is (= ["CREATE INDEX IF NOT EXISTS my_column_idx ON my_table (my_column)"]
             (sql/format {:create-index [:my-column-idx :if-not-exists]
                          :on [:my-table :my-column]})))))

  (testing "create unique index"
    (is (= ["CREATE UNIQUE INDEX my_column_idx ON my_table (my_column)"]
           (sql/format {:create-unique-index :my-column-idx
                        :on [:my-table :my-column]})))))

As long as you don't use :if-not-exists it looks like this is also valid in MySQL and SQL Server. But anything more complicated is vendor-specific.

Is something like this worth including, if I add docs and polish and make a PR? Does adding the :on clause make sense at all, considering the ON keyword is also used for joins?

@seancorfield
Copy link
Owner Author

I wouldn't want to add :on as a clause. The way :join works is [table-spec join-spec] so the equivalent here would be:

{:create-index [:my-column-idx [:my-table :my-column]]}
{:create-index [[:my-column-idx :if-not-exists] [:my-table :my-column]]}

I would prefer not to introduce a separate :create-unique-index but make that also part of the "index spec":

{:create-index [[:unique :my-column-idx] [:my-table :my-column]]}
{:create-index [[:unique :my-column-idx :if-not-exists] [:my-table :my-column]]}

So the single new clause formatter would destructure the args into index-spec and table-column, lift :unique out of index-spec if present (to determine :index vs :unique-index) and then format for :create and also build the ON part and append it to the SQL.

If this satisfies your use case, I'm happy to add that to core and document it. Then I'll deal with any additional cases as they come up later 🙂

@seancorfield seancorfield reopened this Dec 7, 2023
@seancorfield seancorfield added enhancement and removed needs analysis I need to think about this! labels Dec 7, 2023
@dancek
Copy link
Contributor

dancek commented Dec 8, 2023

Yeah, that looks good. I'll add support for multiple columns and expressions instead of columns (PostgreSQL specific) and make a PR of that.

dancek added a commit to dancek/honeysql that referenced this issue Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants