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

Implement sqliteBlobOpen for pdo_sqlite, matching SQLite3 extension #2698

Closed
wants to merge 3 commits into from

Conversation

bohwaz
Copy link
Contributor

@bohwaz bohwaz commented Aug 21, 2017

This adds one new method PDO::sqliteOpenBlob(string table, string column, int rowid [, string dbname [, int flags]])

flags can be either PDO::SQLITE_OPEN_READONLY or PDO::SQLITE_OPEN_READWRITE

This is matching the implementation done in the SQLite3 extension (as SQLite3::openBlob).

stream->eof = 0;
return 0;
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: since all blocks return early, else and else if are not needed

@adambaratz
Copy link
Contributor

A new method is an API change to me, so an RFC would be warranted. I'm reluctant to add driver-specific methods, since that seems opposed to PDO's driver-agnostic API, but that's not to say we couldn't hash something out.

@bohwaz
Copy link
Contributor Author

bohwaz commented Aug 22, 2017

Let's get the discussion about RFC on internals :)

@krakjoe
Copy link
Member

krakjoe commented Sep 6, 2017

Can we know the status here, link to internals (externals.io) discussion would be useful ?

@bohwaz
Copy link
Contributor Author

bohwaz commented Sep 6, 2017

Hey Joe, thanks for approving my account on the wiki.

Discussion is here: https://externals.io/message/100268

I'll do a RFC proposal asap. I didn't think it would be necessary as this is not changing much, but as some people objected on internals, not against my patch but against the existing PDO way of doing things, I'll do a RFC for that patch.

I will update this pull request to remove the conflict with #2734 and do another one for the implementation of sqlite_stmt_readonly. Probably next week or so.

@php-pulls
Copy link

Comment on behalf of cmb at php.net:

Since the respective RFC has been declined, I'm closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants