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

Tests: avoid pgsql bugs of 9.5 and 10.0 #15266

Conversation

bscheshirwork
Copy link
Contributor

This is only compare with actual data from database schema.

All of other usercase, who related to this pgsql errors is broken.

note: Wait for fix it in source and set top versions who have errors in next PR's.

Q A
Is bugfix? yes/no
New feature? no
Breaks BC? no
Tests pass? yes
Fixed issues #15265 #15247

@bscheshirwork
Copy link
Contributor Author

Also I send info about this errors to pgsql-bugs

@samdark
Copy link
Member

samdark commented Dec 2, 2017

Would you please post links to posgresql issue tracker bugs here?

@samdark
Copy link
Member

samdark commented Dec 2, 2017

Should not be merged before pgsql isssues are fixed. It's hiding the issue, not fixing it.

@polobo
Copy link

polobo commented Jan 24, 2018

Here's a link to the thread on the PostgreSQL online mailing list browser - there is no dedicated bug tracker.

https://www.postgresql.org/message-id/flat/20180124222007.GQ17109%40momjian.us#20180124222007.GQ17109@momjian.us

No one choose to research this report - and whether its a bug or simple compatibility breakage would depend on findings - partly because the reported complaint has not been reduced to something that is demonstrably PostgreSQL-output related.

What would help move this forward in PostgreSQL would be for someone to write up a self-contained script generates data question and then run that script against 9.4, 9.5, 9.6, and 10. Post the results of each along with the script to the pgsql-bugs@lists.postgresql.org (or replying to the linked thread if you happen to have it in your inbox). At least 9.4/9.5 where the smallint problem reported seems to have appeared. At that point the differences in output can either be explained or fixed depending on what caused it in the first place.

David J.

@polobo
Copy link

polobo commented Jan 24, 2018

FYI - I've done a limited version example of what I think you are talking about (smallint, 9.3 vs 9.6) and posted it to the original bug report thread.

@bscheshirwork
Copy link
Contributor Author

@polobo Thanks!

@schmunk42
Copy link
Contributor

From the answer of Tom Lane in the linked thread above:

Oh, right, this was an intentional change awhile back.
[...]
(digs...) It was changed in this 9.5-era commit:

So, I don't think this will be "fixed" in upstream in Postgres.

What can we do about it? Should Yii convert that?


Did something change in the core about that, btw?! I can not reproduce the failing smallint test locally at the moment.

@samdark
Copy link
Member

samdark commented Jan 25, 2018

@bscheshirwork would be great if you'll re-check if the issue in still in master. Thanks.

@schmunk42
Copy link
Contributor

schmunk42 commented Jan 25, 2018

At least 9.4/9.5 where the smallint problem reported seems to have appeared.


[edited] Umm, so ... from the tests below 9.4 is OK, the "smallint" bug exists since 9.5.

@schmunk42
Copy link
Contributor

Postgres test status

All tested against eb57d41

Broken (only "smallint" bug)

OK

The error with "float_col2" seems to have disappeared.

I am really lost on this issue :(

@polobo
Copy link

polobo commented Jan 25, 2018

Short answer: The current behavior, since 9.5, is the correct behavior. Those single quotes that now appear are going to stay. 9.4 and earlier (lacking the isngle quotes) was wrong - but won't be fixed in that (or earlier) realeases. If your code must support both outputs you will need to decide on the best way to do that for your project.

@schmunk42
Copy link
Contributor

Isn't the question (or the test) about how we "see" these values on the PHP side?

I.e. Mysql has a type map, which explicitly defines 'smallint' => self::TYPE_SMALLINT, - do we also need that for Postgres?

I do not use Postgres, but from my understanding I'd expect a smallint column to be an integer in PHP.

@polobo
Copy link

polobo commented Jan 25, 2018

It boils down to why you are choosing to parse the default expression in the first place instead of just passing through whatever the database returns. I suppose trimming off the cast would make sense but take the value being cast as-is. IOW, the failing test should probably be dropped altogether. But alas I know PostgreSQL but not yii2 so there may be someone feature that depends on the specifics.

@polobo
Copy link

polobo commented Jan 25, 2018

To make it clear, your are not really parsing a returned value of a smallint, you are parsing the expression that someone happened to attach to a create table command. In many ways all that is is a string expression whose evaluated result is cast to the column type. And the client has little means to evaluate the provided expression, only the server can do that. For the client the expression should just be documentation. And you should not need to parse documentation.

@samdark samdark modified the milestones: 2.0.16, 2.0.14 Feb 4, 2018
@samdark samdark closed this Feb 6, 2018
@bscheshirwork bscheshirwork deleted the tests-pgsql-avoid-latest-version-bugs branch February 19, 2018 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants