Skip to content
This repository was archived by the owner on May 2, 2023. It is now read-only.

Conversation

@RoderickJDunn
Copy link
Contributor

Added support for inserting optimizer hints into Oracle and MySQL queries.

For example, with the following Oracle table

> c = Compiler(Oracle(host='host', database='db', thread_count=2, user='user'))
> t = table("point")

You can now create a select query with a PARALLEL optimization hint as follows

> q = c.compile(t.where(this.b > 10).select(this.b, optimizer_hints='PARALLEL(a 16)'))
> print(q)
SELECT /*+ PARALLEL(a 16) */ "b" FROM "point" WHERE ("b" > 10)

@erezsh
Copy link
Contributor

erezsh commented Feb 13, 2023

Cool! But why not make the base raise NotImplementedError? That makes more sense to me than overriding every database that doesn't support it.

@RoderickJDunn
Copy link
Contributor Author

I went back and forth but ended up with this because the /*+ */ syntax is supported by 3 databases so figured I could reuse the same builder for all those. I see mssql isn't supported yet though so you're right, it makes more sense to raise the error from base.

Also fixing the unit test issues now

@erezsh
Copy link
Contributor

erezsh commented Feb 13, 2023

Sounds good. If you want to re-use this method, you can either use mixin inheritance, or just create a module function and call it from each class.

@RoderickJDunn
Copy link
Contributor Author

RoderickJDunn commented Feb 13, 2023

I implemented it using a mixin as you suggested, but its not 100% clear to me what the MIXINS property does in Dialects. Is it basically a list of mixins that are supported by the dialect, but only if a user chooses to load them it explicitly?

@erezsh
Copy link
Contributor

erezsh commented Feb 13, 2023

Yeah, sorry, I accidentally mislead you. I meant a mixin as a concept (https://en.wikipedia.org/wiki/Mixin), not the specific AbstractMixin mechanism, which is a bit of an overkill for this feature.

But it's not bad that you did it this way. Maybe it's even for the best!

Here is how users can use mixins: https://sqeleton.readthedocs.io/en/latest/intro.html#dialect-mixins

And in the future I plan to make that interface a little bit nicer too.

@RoderickJDunn
Copy link
Contributor Author

TY, I'll leave this as is for now then.

erezsh added a commit that referenced this pull request Feb 24, 2023
@erezsh erezsh merged commit 7457caa into datafold:master Feb 24, 2023
@erezsh
Copy link
Contributor

erezsh commented Feb 24, 2023

Merged. Sorry for the delay, I was on vacation.

Thank you for the contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants