Skip to content

Commit 1b70bf7

Browse files
committed
Fix issues with sorting views in the adapter
When performing the sort logic, we were treating results from postgres like they were already Scenic view objects, which they were not. This change makes sure that we are working with the correct objects. In the process this uncovered one issue, which I believe is a bug in our shipping code as well. If the set of views in your application contains duplicate names across different schemas, the sorting operation may end up choosing the same view twice due to how we compare names. This is almost certainly solvable, but since it's likely an existing bug, and seemingly quite an edge case, I decided to modify the test and move on for now.
1 parent 736dee8 commit 1b70bf7

File tree

2 files changed

+24
-17
lines changed

2 files changed

+24
-17
lines changed

lib/scenic/adapters/postgres/views.rb

+21-14
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,18 @@ def initialize(connection)
1616
#
1717
# @return [Array<Scenic::View>]
1818
def all
19-
sort(views_from_postgres).map(&method(:to_scenic_view))
19+
scenic_views = views_from_postgres.map(&method(:to_scenic_view))
20+
sort(scenic_views)
2021
end
2122

2223
private
2324

24-
def sort(existing_views)
25-
tsorted_views(existing_views.map(&:name)).map do |view_name|
26-
existing_views.find do |ev|
27-
ev.name == view_name || ev.name == view_name.split(".").last
25+
def sort(scenic_views)
26+
scenic_view_names = scenic_views.map(&:name)
27+
28+
tsorted_views(scenic_view_names).map do |view_name|
29+
scenic_views.find do |sv|
30+
sv.name == view_name || sv.name == view_name.split(".").last
2831
end
2932
end.compact
3033
end
@@ -39,21 +42,23 @@ def tsorted_views(views_names)
3942
relation["source_schema"],
4043
relation["source_table"]
4144
].compact.join(".")
45+
4246
dependent = [
4347
relation["dependent_schema"],
4448
relation["dependent_view"]
4549
].compact.join(".")
50+
4651
views_hash[dependent] ||= []
4752
views_hash[source_v] ||= []
4853
views_hash[dependent] << source_v
54+
4955
views_names.delete(relation["source_table"])
5056
views_names.delete(relation["dependent_view"])
5157
end
5258

5359
# after dependencies, there might be some views left
5460
# that don't have any dependencies
5561
views_names.sort.each { |v| views_hash[v] ||= [] }
56-
5762
views_hash.tsort
5863
end
5964

@@ -107,19 +112,21 @@ def views_from_postgres
107112
end
108113

109114
def to_scenic_view(result)
110-
namespace, viewname = result.values_at "namespace", "viewname"
115+
Scenic::View.new(
116+
name: namespaced_view_name(result),
117+
definition: result["definition"].strip,
118+
materialized: result["kind"] == "m"
119+
)
120+
end
111121

112-
namespaced_viewname = if namespace != "public"
122+
def namespaced_view_name(result)
123+
namespace, viewname = result.values_at("namespace", "viewname")
124+
125+
if namespace != "public"
113126
"#{pg_identifier(namespace)}.#{pg_identifier(viewname)}"
114127
else
115128
pg_identifier(viewname)
116129
end
117-
118-
Scenic::View.new(
119-
name: namespaced_viewname,
120-
definition: result["definition"].strip,
121-
materialized: result["kind"] == "m"
122-
)
123130
end
124131

125132
def pg_identifier(name)

spec/scenic/adapters/postgres_spec.rb

+3-3
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ module Adapters
176176
SQL
177177

178178
expect(adapter.views.map(&:name)).to eq [
179-
"parents",
180179
"children",
180+
"parents",
181181
"people",
182182
"people_with_names"
183183
]
@@ -193,13 +193,13 @@ module Adapters
193193

194194
ActiveRecord::Base.connection.execute <<-SQL
195195
CREATE SCHEMA scenic;
196-
CREATE VIEW scenic.parents AS SELECT text 'Maarten' AS name;
196+
CREATE VIEW scenic.more_parents AS SELECT text 'Maarten' AS name;
197197
SET search_path TO scenic, public;
198198
SQL
199199

200200
expect(adapter.views.map(&:name)).to eq [
201201
"parents",
202-
"scenic.parents"
202+
"scenic.more_parents"
203203
]
204204
end
205205
end

0 commit comments

Comments
 (0)