Skip to content

added new memeber SqlState to PostgreSQLException and made use of it.#4099

Merged
aleks-f merged 3 commits intopocoproject:develfrom
omerbrandis:PostgresSQLSTATE
Oct 20, 2023
Merged

added new memeber SqlState to PostgreSQLException and made use of it.#4099
aleks-f merged 3 commits intopocoproject:develfrom
omerbrandis:PostgresSQLSTATE

Conversation

@omerbrandis
Copy link
Copy Markdown
Contributor

@omerbrandis omerbrandis commented Jul 27, 2023

added support for SqlState in data::poco::postgresql.

tested locally against postgresql 15.3.
was not able to find a suitable "location" for adding a "test" to the repo.

tried to make it as simple as I could:

added a data member _SqlState to PostgreSQLException,
added a getter SqlState(),
added ctor PostgreSQLException(const std::string& aMessage,const char * pAnSqlState);

added ctor StatementException::StatementException(const std::string& aMessage,const char pAnSqlState*);

modified StatementExecutor::prepare & StatementExecutor::execute(),
to make use of the new ctor for StatementException, such that the SqlState will be made available to the catcher of the exception.

:-)

@omerbrandis omerbrandis mentioned this pull request Jul 27, 2023
Closed
@aleks-f
Copy link
Copy Markdown
Member

aleks-f commented Sep 11, 2023

@omerbrandis can you please write a test case for this?

@aleks-f aleks-f added this to the Release 1.13.0 milestone Sep 11, 2023
@omerbrandis
Copy link
Copy Markdown
Contributor Author

omerbrandis commented Sep 11, 2023

Dear alexks-f.

Can you please provide some instruction (or a reference to documentation about ) as to where/how to add a test for this
repo and how to execute it?
( as mentioned before, I did not find information about this , I was unable to do it ( not unwilling )).

thanks,
Omer.

@aleks-f
Copy link
Copy Markdown
Member

aleks-f commented Sep 11, 2023

@omerbrandis tests are here

@omerbrandis
Copy link
Copy Markdown
Contributor Author

omerbrandis commented Sep 12, 2023

Hello @aleks-f

I suggest the following test :
void PostgreSQLTest::testSqlState()
{
if (!_pSession) fail ("Test not available.");

    try
    {
    *_pSession << "syntax error", now;
    }
catch (const Poco::Data::PostgreSQL::PostgreSQLException & Except)
{
    	assertTrue(Except.SqlState() == std::string("42601"));    
    }

}

can you have a look, tell me if it seems sufficient or otherwise.
if so, i'll find how to add this to the original pull request or open a new one.

thanks,
Omer.

@omerbrandis
Copy link
Copy Markdown
Contributor Author

omerbrandis commented Sep 13, 2023

Hello @aleks-f

I've tested the new test on my machine.
i've pushed the new commit to my fork
it appears here in the pull request, so i've made no additional changes to the pull request and have not created a new one.

if there's anything else I should do , plz let me know.
:-)

aleks-f
aleks-f requested changes Sep 13, 2023
View reviewed changes
Data/PostgreSQL/include/Poco/Data/PostgreSQL/PostgreSQLException.h Outdated


private:
char _SqlState[6];
Copy link
Copy Markdown
Member

@aleks-f aleks-f Sep 13, 2023

Choose a reason for hiding this comment

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

naming convention - this is not how we name member variables

Data/PostgreSQL/include/Poco/Data/PostgreSQL/PostgreSQLException.h Outdated
/// throwing it again.

const char* SqlState() const noexcept;
/// Returns the SqlState
Copy link
Copy Markdown
Member

@aleks-f aleks-f Sep 13, 2023

Choose a reason for hiding this comment

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

tabs, not spaces for indentation

Data/PostgreSQL/testsuite/src/PostgreSQLTest.cpp Outdated

try
{
*_pSession << "syntax error", now;
Copy link
Copy Markdown
Member

@aleks-f aleks-f Sep 13, 2023

Choose a reason for hiding this comment

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

tabs please

@aleks-f
Copy link
Copy Markdown
Member

aleks-f commented Sep 13, 2023

https://github.com/pocoproject/poco/blob/devel/CONTRIBUTING.md

@omerbrandis
fixed nameing convention errors. fixed bug in PostgreSQLException::Po…
c90931a 
…stgreSQLException regarding null termination of _sqlState data member
@omerbrandis
Copy link
Copy Markdown
Contributor Author

omerbrandis commented Sep 14, 2023

Hello @aleks-f

I've pushed new version.

Sorry for the naming issues,
(I did review the contribution guidelines before starting on this path, but obviously did not do a good job of it ).

I also fixed a bug that did not appear previously.

hope that's everything,
and if not I'll happily fix the rest...

thanks,
Omer.

@aleks-f aleks-f merged commit 3793c0a into pocoproject:devel Oct 20, 2023
aleks-f pushed a commit that referenced this pull request Nov 23, 2023
@omerbrandis @aleks-f
added new memeber SqlState to PostgreSQLException and made use of it. (…
752e051 
…#4099)

* added new memeber SqlState to PostgreSQLException and made use of it in StatementExecutor

* Added test case testSqlState

* fixed nameing convention errors. fixed bug in PostgreSQLException::PostgreSQLException regarding null termination of _sqlState data member
aleks-f pushed a commit that referenced this pull request Nov 27, 2023
@omerbrandis @aleks-f
added new memeber SqlState to PostgreSQLException and made use of it. (…
64c1707 
…#4099)

* added new memeber SqlState to PostgreSQLException and made use of it in StatementExecutor

* Added test case testSqlState

* fixed nameing convention errors. fixed bug in PostgreSQLException::PostgreSQLException regarding null termination of _sqlState data member
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aleks-f aleks-f aleks-f approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Release 1.13.0

Development

Successfully merging this pull request may close these issues.

2 participants

@omerbrandis @aleks-f