From e19ec1c7ea945eadfcb8597938c8bd6be0a4baff Mon Sep 17 00:00:00 2001 From: Dmitrii Puzikov Date: Wed, 4 Nov 2026 16:10:09 +0100 Subject: [PATCH 1/3] fix(redshift): Read Redshift BOOLEAN columns as true/false with driver 2.x redshift-jdbc42 2.2.9 returns BOOLEAN as "t"/"f" from getString(). The 1.x driver returned "true"/"false". realize_query writes every column with get_string, so brick input with a BOOLEAN column changed from true/false to t/f. realize_query now reads BIT/BOOLEAN columns with get_boolean and writes "true"/"false", or an empty value for NULL. JIRA: GRIF-1220 Co-Authored-By: Claude Opus 5.5 --- .../redshift/redshift_client.rb | 17 +++++++++- .../redshift/redshift_client_spec.rb | 33 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/lib/gooddata/cloud_resources/redshift/redshift_client.rb b/lib/gooddata/cloud_resources/redshift/redshift_client.rb index 09ddf2abd..89db2d0fa 100644 --- a/lib/gooddata/cloud_resources/redshift/redshift_client.rb +++ b/lib/gooddata/cloud_resources/redshift/redshift_client.rb @@ -57,9 +57,10 @@ def realize_query(query, _params) result = statement.get_result_set metadata = result.get_meta_data col_count = metadata.column_count + boolean_columns = boolean_column_indexes(metadata) CSV.open(filename, 'wb') do |csv| csv << Array(1..col_count).map { |i| metadata.get_column_name(i) } # build the header - csv << Array(1..col_count).map { |i| result.get_string(i)&.to_s } while result.next + csv << Array(1..col_count).map { |i| column_value(result, i, boolean_columns) } while result.next end end end @@ -98,6 +99,20 @@ def connect private + # Driver 2.x getString() returns BOOLEAN as "t"/"f"; keep "true"/"false" + def boolean_column_indexes(metadata) + Array(1..metadata.column_count).select do |i| + [java.sql.Types::BIT, java.sql.Types::BOOLEAN].include?(metadata.get_column_type(i)) + end + end + + def column_value(result, index, boolean_columns) + return result.get_string(index)&.to_s unless boolean_columns.include?(index) + + value = result.get_boolean(index) + result.was_null ? nil : value.to_s + end + def build_url(url, database) url_parts = url.split('?') url_path = url_parts[0].chomp('/') diff --git a/spec/unit/cloud_resources/redshift/redshift_client_spec.rb b/spec/unit/cloud_resources/redshift/redshift_client_spec.rb index 466d93770..3197b4ac1 100644 --- a/spec/unit/cloud_resources/redshift/redshift_client_spec.rb +++ b/spec/unit/cloud_resources/redshift/redshift_client_spec.rb @@ -66,5 +66,38 @@ def connect_properties expect(props.getProperty('SecretAccessKey')).to eq 'secret_key' end end + + describe '#realize_query' do + let(:authentication) { { 'basic' => { 'userName' => 'basic_user', 'password' => 'secret' } } } + let(:metadata) { double('metadata', column_count: 3) } + let(:result) { double('result') } + let(:statement) { double('statement', execute: true, get_result_set: result) } + let(:connection) { double('connection', create_statement: statement, close: nil) } + + before do + allow(driver).to receive(:connect).and_return(connection) + allow(result).to receive(:get_meta_data).and_return(metadata) + allow(metadata).to receive(:get_column_name) { |i| %w[name active deleted][i - 1] } + allow(metadata).to receive(:get_column_type) do |i| + [java.sql.Types::VARCHAR, java.sql.Types::BIT, java.sql.Types::BOOLEAN][i - 1] + end + allow(result).to receive(:next).and_return(true, true, false) + allow(result).to receive(:get_string).with(1).and_return('Alice', 'Bob') + # Driver 2.x getString() returns BOOLEAN as "t"/"f" + allow(result).to receive(:get_string).with(2).and_return('t', 'f') + allow(result).to receive(:get_boolean).with(2).and_return(true, false) + allow(result).to receive(:get_boolean).with(3).and_return(false, false) + allow(result).to receive(:was_null).and_return(false, false, false, true) + end + + it 'writes BOOLEAN columns as true/false and NULL as empty' do + Dir.mktmpdir do |dir| + Dir.chdir(dir) do + rows = CSV.read(subject.realize_query('SELECT 1', nil)) + expect(rows).to eq [%w[name active deleted], %w[Alice true false], ['Bob', 'false', nil]] + end + end + end + end end end From debe4b7b59e7e51afffd66c9e53d3675d00c9395 Mon Sep 17 00:00:00 2001 From: Dmitrii Puzikov Date: Wed, 4 Nov 2026 16:15:52 +0100 Subject: [PATCH 2/3] test(redshift): Move the realize_query spec to its own describe block The new example made the describe block 79 lines long. The configured Metrics/BlockLength limit is 60. A shared context now holds the driver stubs for both blocks. JIRA: GRIF-1220 Co-Authored-By: Claude Opus 5.5 --- .../redshift/redshift_client_spec.rb | 60 ++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/spec/unit/cloud_resources/redshift/redshift_client_spec.rb b/spec/unit/cloud_resources/redshift/redshift_client_spec.rb index 3197b4ac1..6b623c39f 100644 --- a/spec/unit/cloud_resources/redshift/redshift_client_spec.rb +++ b/spec/unit/cloud_resources/redshift/redshift_client_spec.rb @@ -7,7 +7,7 @@ if RUBY_PLATFORM =~ /java/ require 'gooddata/cloud_resources/redshift/redshift_client' - describe GoodData::CloudResources::RedshiftClient do + shared_context 'redshift client with stubbed driver' do let(:driver) { double('driver') } let(:driver_class) { double('driver_class', new: driver) } let(:options) do @@ -32,6 +32,10 @@ allow(client).to receive_message_chain('com.amazon.redshift.jdbc42.Driver').and_return(driver_class) client end + end + + describe GoodData::CloudResources::RedshiftClient do + include_context 'redshift client with stubbed driver' def connect_properties received = nil @@ -66,36 +70,38 @@ def connect_properties expect(props.getProperty('SecretAccessKey')).to eq 'secret_key' end end + end - describe '#realize_query' do - let(:authentication) { { 'basic' => { 'userName' => 'basic_user', 'password' => 'secret' } } } - let(:metadata) { double('metadata', column_count: 3) } - let(:result) { double('result') } - let(:statement) { double('statement', execute: true, get_result_set: result) } - let(:connection) { double('connection', create_statement: statement, close: nil) } + describe GoodData::CloudResources::RedshiftClient, '#realize_query' do + include_context 'redshift client with stubbed driver' - before do - allow(driver).to receive(:connect).and_return(connection) - allow(result).to receive(:get_meta_data).and_return(metadata) - allow(metadata).to receive(:get_column_name) { |i| %w[name active deleted][i - 1] } - allow(metadata).to receive(:get_column_type) do |i| - [java.sql.Types::VARCHAR, java.sql.Types::BIT, java.sql.Types::BOOLEAN][i - 1] - end - allow(result).to receive(:next).and_return(true, true, false) - allow(result).to receive(:get_string).with(1).and_return('Alice', 'Bob') - # Driver 2.x getString() returns BOOLEAN as "t"/"f" - allow(result).to receive(:get_string).with(2).and_return('t', 'f') - allow(result).to receive(:get_boolean).with(2).and_return(true, false) - allow(result).to receive(:get_boolean).with(3).and_return(false, false) - allow(result).to receive(:was_null).and_return(false, false, false, true) + let(:authentication) { { 'basic' => { 'userName' => 'basic_user', 'password' => 'secret' } } } + let(:metadata) { double('metadata', column_count: 3) } + let(:result) { double('result') } + let(:statement) { double('statement', execute: true, get_result_set: result) } + let(:connection) { double('connection', create_statement: statement, close: nil) } + + before do + allow(driver).to receive(:connect).and_return(connection) + allow(result).to receive(:get_meta_data).and_return(metadata) + allow(metadata).to receive(:get_column_name) { |i| %w[name active deleted][i - 1] } + allow(metadata).to receive(:get_column_type) do |i| + [java.sql.Types::VARCHAR, java.sql.Types::BIT, java.sql.Types::BOOLEAN][i - 1] end + allow(result).to receive(:next).and_return(true, true, false) + allow(result).to receive(:get_string).with(1).and_return('Alice', 'Bob') + # Driver 2.x getString() returns BOOLEAN as "t"/"f" + allow(result).to receive(:get_string).with(2).and_return('t', 'f') + allow(result).to receive(:get_boolean).with(2).and_return(true, false) + allow(result).to receive(:get_boolean).with(3).and_return(false, false) + allow(result).to receive(:was_null).and_return(false, false, false, true) + end - it 'writes BOOLEAN columns as true/false and NULL as empty' do - Dir.mktmpdir do |dir| - Dir.chdir(dir) do - rows = CSV.read(subject.realize_query('SELECT 1', nil)) - expect(rows).to eq [%w[name active deleted], %w[Alice true false], ['Bob', 'false', nil]] - end + it 'writes BOOLEAN columns as true/false and NULL as empty' do + Dir.mktmpdir do |dir| + Dir.chdir(dir) do + rows = CSV.read(subject.realize_query('SELECT 1', nil)) + expect(rows).to eq [%w[name active deleted], %w[Alice true false], ['Bob', 'false', nil]] end end end From 6a98d80d25fb7aebe00b49c4b9087f3db87dee54 Mon Sep 17 00:00:00 2001 From: Dmitrii Puzikov Date: Wed, 4 Nov 2026 16:44:05 +0100 Subject: [PATCH 3/3] test(redshift): Stub the loggers in the realize_query spec realize_query writes to GoodData.gd_logger and GoodData.logger. Other specs leave rspec doubles in these global loggers, so the example failed in the full suite with a leaked double error. JIRA: GRIF-1220 Co-Authored-By: Claude Opus 5.5 --- spec/unit/cloud_resources/redshift/redshift_client_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/unit/cloud_resources/redshift/redshift_client_spec.rb b/spec/unit/cloud_resources/redshift/redshift_client_spec.rb index 6b623c39f..1e1d86544 100644 --- a/spec/unit/cloud_resources/redshift/redshift_client_spec.rb +++ b/spec/unit/cloud_resources/redshift/redshift_client_spec.rb @@ -82,6 +82,9 @@ def connect_properties let(:connection) { double('connection', create_statement: statement, close: nil) } before do + # Other specs leave doubles in the global loggers + allow(GoodData).to receive(:gd_logger).and_return(double('gd_logger', info: nil)) + allow(GoodData).to receive(:logger).and_return(double('logger', info: nil)) allow(driver).to receive(:connect).and_return(connection) allow(result).to receive(:get_meta_data).and_return(metadata) allow(metadata).to receive(:get_column_name) { |i| %w[name active deleted][i - 1] }