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

IDEA(dbal): A set of interfaces for constructing queries #7562

Closed
wants to merge 3 commits into from

Conversation

ewinslow
Copy link
Contributor

These interfaces allow us to construct most basic queries within Elgg
in a programmatic and safe way. Have not attempted to find out where
the more complicated entities queries could be converted to this,
but it does seem pretty generic and able to handle quite a range, so
I would be remiss not to try.

The major downside of all this of course is that it would be mostly custom
implementation. The main attempt at "innovation" is using pass-by-reference
to alias tables instead of strings. This guarantees that all aliases are unique
and allows you to refer to columns/tables in the query by references rather
than strings.

I didn't look outside of doctrine for a query builder interface but if you know any others that might be close to this, that would be appreciated. Hopefully there is enough documentation here to give the gist of the API and generate some good discussion.

These interfaces allow us to construct most basic queries within Elgg
in a programmatic and safe way. Have not attempted to find out where
the more complicated entities queries could be converted to this,
but it does seem pretty generic and able to handle quite a range, so
I would be remiss not to try.

The major downside of all this of course is that it would be mostly custom
implementation. The main attempt at "innovation" is using pass-by-reference
to alias tables instead of strings. This guarantees that all aliases are unique
and allows you to refer to columns/tables in the query by references rather
than strings.
@mrclay
Copy link
Member

mrclay commented Nov 30, 2014

This looks fine. I'm a little meh on the ROI of rolling our own vs pulling in Doctrine DBAL. I'm not sure SQL is a big problem for us, and to the people that need help writing valid SQL I'd prefer to teach SQL.

@mrclay
Copy link
Member

mrclay commented Nov 30, 2014

If we had a simpler mechanism for auto-prefixing tables (like Drupal's) our queries would be significantly more readable. I believe Drupal has to parse queries to do this however.

@ewinslow
Copy link
Contributor Author

Don't forget the ability to rewrite in various SQL dialects (actually...
how different are they really?).

I think I'm also looking forward to not needing all the sanitize string and
sanitize int calls... I'm still paranoid we've forgotten one somewhere...

On Sat, Nov 29, 2014, 4:05 PM Steve Clay notifications@github.com wrote:

If we had a simpler mechanism for auto-prefixing tables (like Drupal's)
our queries would be significantly more readable. I believe Drupal has to
parse queries to do this however.


Reply to this email directly or view it on GitHub
#7562 (comment).

@Srokap Srokap added this to the Elgg 1.10.0 milestone Nov 30, 2014
Added some more comments to Elgg\Database\* classes to demonstrate intended conversion
@ewinslow ewinslow modified the milestone: Elgg 1.10.0 Dec 1, 2014
…Statement (which feels like the easiest one to start with).
@ewinslow
Copy link
Contributor Author

ewinslow commented Jan 5, 2015

Too much overhead. Not enough benefit

@ewinslow ewinslow closed this Jan 5, 2015
@Srokap Srokap removed the in progress label Jan 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants