[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

Support running PrivateBin on OCI (Oracle Database), Part 2 #872

Merged
merged 27 commits into from
Jan 31, 2022
Merged

Support running PrivateBin on OCI (Oracle Database), Part 2 #872

merged 27 commits into from
Jan 31, 2022

Conversation

elrido
Copy link
Contributor
@elrido elrido commented Jan 23, 2022 •
edited
Loading

Closes #868, supersedes #871.

  • fix CLOB in comments

rugk
rugk requested changes Jan 23, 2022
View reviewed changes
Copy link
Member
@rugk rugk left a comment

Choose a reason for hiding this comment

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

Some minor stuff (some things were already in the old PR)

lib/Data/Database.php Show resolved Hide resolved
lib/Data/Database.php Show resolved Hide resolved
lib/Data/Database.php Show resolved Hide resolved
lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Show resolved Hide resolved
@elrido
Copy link
Contributor Author
elrido commented Jan 24, 2022

Ok, that should now address most of the above points. We still need some feedback from @austinhuang0131 if this does actually work on Oracle or still has some issues.

austinhuang0131
austinhuang0131 suggested changes Jan 24, 2022
View reviewed changes
Copy link
Contributor
@austinhuang0131 austinhuang0131 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

(Ignore, apparently it creates new tables where the names are lowercase, whereas the original tables are uppercase)

@elrido
Copy link
Contributor Author
elrido commented Jan 24, 2022

ORA-06550: line 9, column 17:
PLS-00103: Encountered the symbol "END" when expecting one of the following:

Could you double check if this shortened function actually works and is idempotent in Oracle SQL, i.e. by applying it repeatedly on the existing table?

PrivateBin/lib/Data/Database.php

Lines 828 to 836 in 0b6af67

'declare
already_exists exception;
columns_indexed exception;
pragma exception_init( already_exists, -955 );
pragma exception_init(columns_indexed, -1408);
begin
execute immediate \'create index "comment_parent" on "' . self::_sanitizeIdentifier('comment') . '" ("pasteid")\';
exception
end'

The reference you gave me had some additional lines - I had hoped to suppress the output, but maybe they are required to give the exception branch something to do?

austinhuang0131
austinhuang0131 suggested changes Jan 24, 2022
View reviewed changes
Copy link
Contributor
@austinhuang0131 austinhuang0131 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Comments still don't load properly (last comment's content visually override earlier ones).

lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Outdated Show resolved Hide resolved
elrido and others added 5 commits January 24, 2022 21:36
@elrido @austinhuang0131
Basically it wants a non-empty catch statement
b8e8755 
Co-authored-by: Austin Huang <im@austinhuang.me>
@elrido @austinhuang0131
this fixes the comment display issue
0c4852c 
Co-authored-by: Austin Huang <im@austinhuang.me>
@elrido @austinhuang0131
handle LIMIT in oci
535f038 
Co-authored-by: Austin Huang <im@austinhuang.me>
@elrido @austinhuang0131
Throws ORA-00942: table or view does not exist otherwise
55db942 
Co-authored-by: Austin Huang <im@austinhuang.me>
@elrido @austinhuang0131
inserting CLOB absolutely requires a length argument
f4438a0 
Co-authored-by: Austin Huang <im@austinhuang.me>
elrido
elrido commented Jan 24, 2022
View reviewed changes
lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Show resolved Hide resolved
austinhuang0131
austinhuang0131 previously approved these changes Jan 25, 2022
View reviewed changes
Copy link
Contributor
@austinhuang0131 austinhuang0131 left a comment

Choose a reason for hiding this comment

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

The branch is now running in its current state at https://bin.bus-hit.me. I have generally tested it (create/read paste with password, upload attachment, commenting, deleting) and it's working.

rugk
rugk previously approved these changes Jan 25, 2022
View reviewed changes
Copy link
Member
@rugk rugk left a comment

Choose a reason for hiding this comment

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

Minor stuff only, LGTM otherwise.

lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Show resolved Hide resolved
@elrido elrido dismissed stale reviews from rugk and austinhuang0131 via 1d20eee January 26, 2022 04:28
@elrido elrido added this to the Release 1.4 - improved UI milestone Jan 26, 2022
@elrido
Copy link
Contributor Author
elrido commented Jan 29, 2022

Can this get merged?

rugk
rugk previously approved these changes Jan 30, 2022
View reviewed changes
Copy link
Member
@rugk rugk left a comment

Choose a reason for hiding this comment

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

Code-wise for me this LGTM.

r4sas
r4sas reviewed Jan 30, 2022
View reviewed changes
lib/Data/Database.php Outdated Show resolved Hide resolved
lib/Data/Database.php Show resolved Hide resolved
@r4sas
Copy link
Member
r4sas commented Jan 30, 2022 •
edited
Loading

Only one question about modified parts - usage of different quotes. But if I correctly understand, "'s used because of variables inside string in second comment.

@elrido elrido merged commit 1f87216 into master Jan 31, 2022
@elrido elrido deleted the austinhuang0131-master branch January 31, 2022 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austinhuang0131 austinhuang0131 austinhuang0131 left review comments

@r4sas r4sas r4sas approved these changes

@rugk rugk rugk left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release 1.4 - improved UI
Development

Successfully merging this pull request may close these issues.

Running PrivateBin on Oracle database
4 participants
@elrido @r4sas @rugk @austinhuang0131