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

Refs #2079. Do not pass $this->query_extra to pg_dump. #2282

Closed

Conversation

xurizaemon
Copy link

No description provided.

Copy link

@Renrhaf Renrhaf left a comment

Choose a reason for hiding this comment

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

Works fine, thanks ! The password may be asked if no .pgfile is set (See https://www.drupal.org/node/438828#comment-5020060)

@RoSk0
Copy link
Contributor

RoSk0 commented Dec 21, 2017

Can we please have this PR updated.

@RoSk0
Copy link
Contributor

RoSk0 commented Dec 21, 2017

I wouldn't agree that changes in this PR are correct.
It works, yes, but removing default value from that check makes PostgreSQL integration behave differently to other supported DBs and that is bad.
I think we should either remove $query_extra field from Sqlpgsql class completely(I think preferred ) and behave as the rest or set to empty string by default.

@xurizaemon
Copy link
Author

👋 hi kirill!

Agree that this isn't perfect solution, your proposal makes sense. Happy for you to submit alternate fix, I'm on leave for now.

@xurizaemon
Copy link
Author

#3267 instead

@xurizaemon xurizaemon closed this Dec 21, 2017
@cafuego
Copy link

cafuego commented Jan 25, 2018

Please re-open this one, the alternative fix looks to break more than it fixes for me.

@xurizaemon
Copy link
Author

xurizaemon commented Jan 25, 2018

Done @cafuego 👋

Don't they use Postgres in the northern hemisphere? It's just us here.

@weitzman
Copy link
Member

I made the same change in mysql today so it makes sense to do it here. My deepest apologies for the long delay. We are in need of a Postgres maintainer.

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

6 participants