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

Database: Initialize xorm with an empty schema for postgres #17357

Merged
merged 2 commits into from
Jun 3, 2019
Merged

Database: Initialize xorm with an empty schema for postgres #17357

merged 2 commits into from
Jun 3, 2019

Conversation

marefr
Copy link
Contributor

@marefr marefr commented May 29, 2019

What this PR does / why we need it:
xorm introduced some changes in
go-xorm/xorm#824 and
go-xorm/xorm#876 which by default will use
public as the postgres schema and this was a breaking change compared
to before. Grafana has implemented a custom postgres dialect so above
changes wasn't a problem here. However, Grafana's custom database
migration was using xorm dialect to check if the migration table exists
or not.
For those using a custom search_path (schema) in postgres configured on
server, database or user level the migration table check would not find
the migration table since it was looking in public schema due to xorm
changes above. This had the consequence that Grafana's database
migration failed the second time since migration had already run
migrations in another schema.
This change will make xorm use an empty default schema for postgres and
by that mimic the functionality of how it was functioning before
xorm's changes above.

Which issue(s) this PR fixes:
Fixes #16720

Special notes for your reviewer:

@marefr
db: initialize xorm with an empty schema for postgres
184b8f6 
xorm introduced some changes in
go-xorm/xorm#824 and
go-xorm/xorm#876 which by default will use
public as the postgres schema and this was a breaking change compared
to before. Grafana has implemented a custom postgres dialect so above
changes wasn't a problem here. However, Grafana's custom database
migration was using xorm dialect to check if the migration table exists
or not.
For those using a custom search_path (schema) in postgres configured on
server, database or user level the migration table check would not find
the migration table since it was looking in public schema due to xorm
changes above. This had the consequence that Grafana's database
migration failed the second time since migration had already run
migrations in another schema.
This change will make xorm use an empty default schema for postgres and
by that mimic the functionality of how it was functioning before
xorm's changes above.
@marefr marefr added this to the 6.2.2 milestone May 29, 2019
@marefr marefr requested review from torkelo and bergquist and removed request for torkelo May 29, 2019 16:17
@marefr @bergquist
add comment about DefaultPostgresSchema
0081fae 
Co-Authored-By: Carl Bergquist <carl@grafana.com>
@marefr marefr requested a review from bergquist June 3, 2019 14:24
bergquist
bergquist approved these changes Jun 3, 2019
View reviewed changes
Copy link
Contributor

@bergquist bergquist left a comment

Choose a reason for hiding this comment

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

LGTM!

@marefr marefr merged commit b7a9533 into master Jun 3, 2019
@marefr marefr deleted the 16720_fix branch June 3, 2019 14:45
ryantxu added a commit to ryantxu/grafana that referenced this pull request Jun 4, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into series-facade
6d70a0b 
* grafana/master: (49 commits)
  Build: specify build flag for `docker-compose up` (grafana#17411)
  Add a @grafana/runtime package with backendSrv interface (grafana#16533)
  Database: Initialize xorm with an empty schema for postgres (grafana#17357)
  docs: configuring custom headers in the dataproxy (grafana#17367)
  Explore: Queries the datasource once per run query and uses DataStreamObserver (grafana#17263)
  Feature: Adds redux action logging toggle from url params (grafana#17368)
  Build: Adds e2e tests back to master workflow with better error messages and with artifacts (grafana#17374)
  Explore: Handle datasources with long names better in ds picker (grafana#17393)
  Annotations: Improve annotation option tooltips (grafana#17384)
  InfluxDB: Fixes single quotes are not escaped (grafana#17398)
  Chore: Bump axios to 0.19.0 (grafana#17403)
  Alerting: golint fixes for alerting (grafana#17246)
  Batch disable users (grafana#17254)
  Chore: Remove unused properties in explore (grafana#17359)
  MySQL/Postgres/MSSQL: Add parsing for day, weeks and year intervals in macros (grafana#13086)
  Security: Prevent csv formula injection attack  (grafana#17363)
  LDAP: remove unused function (grafana#17351)
  Enterprise: remove gofakeit dep (grafana#17344)
  Explore: Update time range before running queries (grafana#17349)
  Build(package.json): improve npm commands (grafana#17022)
  ...
ryantxu added a commit to ryantxu/grafana that referenced this pull request Jun 4, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into extensions
5dcceaa 
* grafana/master: (108 commits)
  Build: specify build flag for `docker-compose up` (grafana#17411)
  Add a @grafana/runtime package with backendSrv interface (grafana#16533)
  Database: Initialize xorm with an empty schema for postgres (grafana#17357)
  docs: configuring custom headers in the dataproxy (grafana#17367)
  Explore: Queries the datasource once per run query and uses DataStreamObserver (grafana#17263)
  Feature: Adds redux action logging toggle from url params (grafana#17368)
  Build: Adds e2e tests back to master workflow with better error messages and with artifacts (grafana#17374)
  Explore: Handle datasources with long names better in ds picker (grafana#17393)
  Annotations: Improve annotation option tooltips (grafana#17384)
  InfluxDB: Fixes single quotes are not escaped (grafana#17398)
  Chore: Bump axios to 0.19.0 (grafana#17403)
  Alerting: golint fixes for alerting (grafana#17246)
  Batch disable users (grafana#17254)
  Chore: Remove unused properties in explore (grafana#17359)
  MySQL/Postgres/MSSQL: Add parsing for day, weeks and year intervals in macros (grafana#13086)
  Security: Prevent csv formula injection attack  (grafana#17363)
  LDAP: remove unused function (grafana#17351)
  Enterprise: remove gofakeit dep (grafana#17344)
  Explore: Update time range before running queries (grafana#17349)
  Build(package.json): improve npm commands (grafana#17022)
  ...
ryantxu added a commit to ryantxu/grafana that referenced this pull request Jun 4, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into transforms
65a6184 
* grafana/master: (142 commits)
  Build: specify build flag for `docker-compose up` (grafana#17411)
  Add a @grafana/runtime package with backendSrv interface (grafana#16533)
  Database: Initialize xorm with an empty schema for postgres (grafana#17357)
  docs: configuring custom headers in the dataproxy (grafana#17367)
  Explore: Queries the datasource once per run query and uses DataStreamObserver (grafana#17263)
  Feature: Adds redux action logging toggle from url params (grafana#17368)
  Build: Adds e2e tests back to master workflow with better error messages and with artifacts (grafana#17374)
  Explore: Handle datasources with long names better in ds picker (grafana#17393)
  Annotations: Improve annotation option tooltips (grafana#17384)
  InfluxDB: Fixes single quotes are not escaped (grafana#17398)
  Chore: Bump axios to 0.19.0 (grafana#17403)
  Alerting: golint fixes for alerting (grafana#17246)
  Batch disable users (grafana#17254)
  Chore: Remove unused properties in explore (grafana#17359)
  MySQL/Postgres/MSSQL: Add parsing for day, weeks and year intervals in macros (grafana#13086)
  Security: Prevent csv formula injection attack  (grafana#17363)
  LDAP: remove unused function (grafana#17351)
  Enterprise: remove gofakeit dep (grafana#17344)
  Explore: Update time range before running queries (grafana#17349)
  Build(package.json): improve npm commands (grafana#17022)
  ...
ryantxu added a commit to ryantxu/grafana that referenced this pull request Jun 4, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into anno-panel
221f65c 
* grafana/master:
  Build: specify build flag for `docker-compose up` (grafana#17411)
  Add a @grafana/runtime package with backendSrv interface (grafana#16533)
  Database: Initialize xorm with an empty schema for postgres (grafana#17357)
bergquist added a commit to jrockway/grafana that referenced this pull request Jun 5, 2019
@bergquist
Merge branch 'master' into jrockway-zipkin
a3b57ba 
* master: (166 commits)
  Cloudwatch: Add AWS DocDB metrics (grafana#17241)
  Provisioning: Support folder that doesn't exist yet in dashboard provisioning (grafana#17407)
  Codestyle: Fix govet issues (grafana#17178)
  @grafana/runtime: expose location update (grafana#17428)
  Fix: Adds context to list of keys that are not part of query (grafana#17423)
  Prometheus: Correctly escape '|' literals in interpolated PromQL variables (grafana#16932)
  Explore: Makes it possible to use a different query field per mode (grafana#17395)
  DataSourceApi: remove ExploreDataSourceApi (grafana#17424)
  Fix: change angular loader paths (grafana#17421)
  Build: specify build flag for `docker-compose up` (grafana#17411)
  Add a @grafana/runtime package with backendSrv interface (grafana#16533)
  Database: Initialize xorm with an empty schema for postgres (grafana#17357)
  docs: configuring custom headers in the dataproxy (grafana#17367)
  Explore: Queries the datasource once per run query and uses DataStreamObserver (grafana#17263)
  Feature: Adds redux action logging toggle from url params (grafana#17368)
  Build: Adds e2e tests back to master workflow with better error messages and with artifacts (grafana#17374)
  Explore: Handle datasources with long names better in ds picker (grafana#17393)
  Annotations: Improve annotation option tooltips (grafana#17384)
  InfluxDB: Fixes single quotes are not escaped (grafana#17398)
  Chore: Bump axios to 0.19.0 (grafana#17403)
  ...
aocenas pushed a commit that referenced this pull request Jun 5, 2019
@marefr @aocenas
Database: Initialize xorm with an empty schema for postgres (#17357)
4d467d0 
xorm introduced some changes in
go-xorm/xorm#824 and
go-xorm/xorm#876 which by default will use
public as the postgres schema and this was a breaking change compared
to before. Grafana has implemented a custom postgres dialect so above
changes wasn't a problem here. However, Grafana's custom database
migration was using xorm dialect to check if the migration table exists
or not.
For those using a custom search_path (schema) in postgres configured on
server, database or user level the migration table check would not find
the migration table since it was looking in public schema due to xorm
changes above. This had the consequence that Grafana's database
migration failed the second time since migration had already run
migrations in another schema.
This change will make xorm use an empty default schema for postgres and
by that mimic the functionality of how it was functioning before
xorm's changes above.
Fixes #16720

Co-Authored-By: Carl Bergquist <carl@grafana.com>
(cherry picked from commit b7a9533)
@aocenas aocenas mentioned this pull request Jun 5, 2019
Merged
aocenas pushed a commit that referenced this pull request Jun 5, 2019
@marefr @aocenas
Database: Initialize xorm with an empty schema for postgres (#17357)
80475fa 
xorm introduced some changes in
go-xorm/xorm#824 and
go-xorm/xorm#876 which by default will use
public as the postgres schema and this was a breaking change compared
to before. Grafana has implemented a custom postgres dialect so above
changes wasn't a problem here. However, Grafana's custom database
migration was using xorm dialect to check if the migration table exists
or not.
For those using a custom search_path (schema) in postgres configured on
server, database or user level the migration table check would not find
the migration table since it was looking in public schema due to xorm
changes above. This had the consequence that Grafana's database
migration failed the second time since migration had already run
migrations in another schema.
This change will make xorm use an empty default schema for postgres and
by that mimic the functionality of how it was functioning before
xorm's changes above.
Fixes #16720

Co-Authored-By: Carl Bergquist <carl@grafana.com>
(cherry picked from commit b7a9533)
@marefr marefr mentioned this pull request Aug 26, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bergquist bergquist bergquist approved these changes

Assignees
No one assigned
Labels
area/backend/db/postgres type/bug
Projects
None yet
Milestone
6.2.2
Development

Successfully merging this pull request may close these issues.

bug with upgrade from 6.0.2 to 6.1.3 or 6.1.4 : column account_id does not exist
3 participants
@marefr @bergquist @aocenas