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 true 'prepare' functionality to Queries #131

Closed
5 tasks
lonnieezell opened this issue Jun 29, 2016 · 6 comments
Closed
5 tasks

Add true 'prepare' functionality to Queries #131

lonnieezell opened this issue Jun 29, 2016 · 6 comments
Assignees
Labels
database Issues or pull requests that affect the database layer enhancement PRs that improve existing functionalities in progress
Milestone

Comments

@lonnieezell
Copy link
Member

lonnieezell commented Jun 29, 2016

Queries should have the ability to be a true prepared statement that can be reused. There are performance reasons NOT to make every query a prepared statement, and the query build/connection already takes care of escaping parameters in the same way that a prepared statement would. However, the user should still be able to use true prepared statements because sometimes it does make sense.

Enhancement Checklist:

  • Component(s) modified
  • Unit testing updated
  • User guide updated
  • Securely signed commits
  • Conforms to style guide
@lonnieezell lonnieezell added enhancement PRs that improve existing functionalities database Issues or pull requests that affect the database layer labels Jun 29, 2016
@lonnieezell lonnieezell added this to the Pre-Alpha 2 milestone Jun 29, 2016
@lonnieezell lonnieezell added the help wanted More help is needed for the proper resolution of an issue or pull request label Jul 11, 2016
@Portaflex
Copy link
Contributor

Portaflex commented Jul 20, 2016

Perhaps we could use bindings to parametrize our query ($1, $2) and pass, say pg_prepare, to the DB. If we mark our query as prepared, we could tell CI4 to ask the DB if it already has our named query. If not, send pg_prepare and execute the query. If yes, just execute the prepared query. We need also a persistent connection.

In this way we would have just one definition for the query, specifying if it should be prepared or not.

@lonnieezell
Copy link
Member Author

I haven't firmly got this one fixed in my head, but the user will need to be able to hold on to the Query object itself so they can re-use it with new data. And will likely need a new method or two to run that query. That's why I think it's going to need a new class (extends the Query class?) to work.

I picture the db connection having a prepare() method which builds the prepared query object, and actually prepares it with the database.

@Portaflex
Copy link
Contributor

Portaflex commented Jul 24, 2016

I've figured out a way to do prepared queries using Postgres. It works 100% of the times without "where" clauses, and 50% of the times with them.

I added one class to Database\Postgres\Prepare.php. And one more method to Database\BaseConnection.php to instantiate the new class.

It is ugly, changes methods from BaseBuilder.php, but works (more or less). Just an idea.

@lonnieezell
Copy link
Member Author

Sorry it took me so long to get back to you. Thanks for taking a stab at that! I don't know that it's ideal, though. I would love to find a way to not have to override anything on the Builder class, so that we didn't mess with anything there. Since it's a query and just has additional capability to send that query multiple times, I think it might work to do something like:

$pQuery = $this->db->prepare(function($db) { 
    return $db->table('foo')->where('ax', 'y')->get();
});

$set1 = $pQuery->run($data1);
$set2 = $pQuery->run($data2);

The trickiest portion of that is that 'get' and similar methods are going to want to actually hit that database, but we might be able to put a flag in there, or something.

Hmm. Will think about it for a few more days but this interface seems promising.

@lonnieezell lonnieezell self-assigned this Jul 29, 2016
@lonnieezell lonnieezell added in progress and removed help wanted More help is needed for the proper resolution of an issue or pull request labels Jul 29, 2016
@lonnieezell
Copy link
Member Author

Have started work on this: #200

@lonnieezell
Copy link
Member Author

Now implemented in 099cc8f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Issues or pull requests that affect the database layer enhancement PRs that improve existing functionalities in progress
Projects
None yet
Development

No branches or pull requests

2 participants