Skip to content

Conversation

@alamashir
Copy link
Contributor

@alamashir alamashir commented Nov 15, 2025

Fixes #47053

Problem

Boolean fields set to False in Google provider connection extras incorrectly return default values. This affects BigQueryHook where use_legacy_sql=False returns True instead.

The bug is in base_google.py:

  • get_field(): Uses or None which treats False as falsy
  • _get_field(): Uses and ... or default logic that returns default when value is False

Root Cause

Code used or None pattern which treats False as falsy. Additionally, couldn't distinguish "parameter not provided" from "parameter explicitly False".

Solution

Implemented sentinel pattern with _UNSET = object() to distinguish unset parameters from explicitly False values.

Changes

  • Added _UNSET sentinel to base_google.py
  • Updated _get_field() to handle sentinel pattern with backward compatibility
  • Modified BigQueryHook.init() to use sentinel pattern

@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues labels Nov 15, 2025
Fixed a bug where boolean fields set to False in Google provider
connection extras were incorrectly returning default values instead.

The issue occurred in two places:
1. get_field() function used 'or None' which treated False as falsy
2. _get_field() method used compound boolean logic that returned
   the default value when the actual value was False

This particularly affected BigQueryHook where use_legacy_sql=False
in connection extras would incorrectly return True.

Changes:
- Modified get_field() to only treat empty strings as None
- Modified _get_field() to explicitly check 'is not None'
- Added tests for False/falsy value handling

Fixes apache#47053
@alamashir alamashir force-pushed the fix-google-provider-false-boolean-47053 branch from c50d3b4 to e03e41c Compare November 15, 2025 17:02
Update mock paths from 'airflow.hooks.base.BaseHook' to
'airflow.providers.common.compat.sdk.BaseHook' to match
the new import structure in Airflow 3.x.
Copy link
Contributor

@shahar1 shahar1 left a comment

Choose a reason for hiding this comment

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

Tests fail :(

@alamashir
Copy link
Contributor Author

alamashir commented Nov 15, 2025

First Issue(which the PR is addressing):

value = get_setting_from_connection()  # Returns False
final_value = value or True  # if value is falsy, use True instead

  # What happens:
  # Step 1: value = False
  # Step 2: Python sees False as "nothing there"
  # Step 3: So it uses True instead
  # Result: False turned into True

looks like there is another issue here
When we tried to fix it, we discovered another issue. There's a difference between:

Scenario A: User doesn't specify anything

  hook = BigQueryHook()  # No setting specified
  # Expected: Check the connection settings
  # If connection says False → use False
  # If connection says nothing → use default (True)

Scenario B: User explicitly specifies a value

  hook = BigQueryHook(use_legacy_sql=True)  # Explicitly set to True
  # Expected: Use True, IGNORE connection settings

The Problem: How do we tell the difference?

  • In both cases, Python just sees use_legacy_sql parameter
  • In Scenario A, it could be None or missing
  • In Scenario B, it's explicitly set to True
  • But what if Scenario A has False in the connection? We need to use False
  • And what if Scenario B explicitly passes False? We need to use False

The challenge: We need a way to say "this parameter wasn't provided at all" vs "this parameter was provided with value False/True/None"

@potiuk @shahar1 thoughts?

@alamashir alamashir requested review from potiuk and shahar1 November 16, 2025 20:52
@potiuk potiuk merged commit c0bce9b into apache:main Nov 16, 2025
82 checks passed
aaron-wolmutt pushed a commit to aaron-wolmutt/airflow that referenced this pull request Nov 20, 2025
…as (apache#58348)

* Fix Google provider to handle False boolean values in connection extras

Fixed a bug where boolean fields set to False in Google provider
connection extras were incorrectly returning default values instead.

The issue occurred in two places:
1. get_field() function used 'or None' which treated False as falsy
2. _get_field() method used compound boolean logic that returned
   the default value when the actual value was False

This particularly affected BigQueryHook where use_legacy_sql=False
in connection extras would incorrectly return True.

Changes:
- Modified get_field() to only treat empty strings as None
- Modified _get_field() to explicitly check 'is not None'
- Added tests for False/falsy value handling

Fixes apache#47053

* Fix test import paths for Airflow 3.x compatibility

Update mock paths from 'airflow.hooks.base.BaseHook' to
'airflow.providers.common.compat.sdk.BaseHook' to match
the new import structure in Airflow 3.x.

* Fix Google provider to respect False boolean values in connection extras

Use sentinel pattern to distinguish unset parameters from explicitly False
values, allowing both connection extras and parameter overrides to work correctly.

* Fix mypy no-redef errors by removing duplicate type annotations

* Fix import order for static checks

* Trigger CI re-run
Copilot AI pushed a commit to jason810496/airflow that referenced this pull request Dec 5, 2025
…as (apache#58348)

* Fix Google provider to handle False boolean values in connection extras

Fixed a bug where boolean fields set to False in Google provider
connection extras were incorrectly returning default values instead.

The issue occurred in two places:
1. get_field() function used 'or None' which treated False as falsy
2. _get_field() method used compound boolean logic that returned
   the default value when the actual value was False

This particularly affected BigQueryHook where use_legacy_sql=False
in connection extras would incorrectly return True.

Changes:
- Modified get_field() to only treat empty strings as None
- Modified _get_field() to explicitly check 'is not None'
- Added tests for False/falsy value handling

Fixes apache#47053

* Fix test import paths for Airflow 3.x compatibility

Update mock paths from 'airflow.hooks.base.BaseHook' to
'airflow.providers.common.compat.sdk.BaseHook' to match
the new import structure in Airflow 3.x.

* Fix Google provider to respect False boolean values in connection extras

Use sentinel pattern to distinguish unset parameters from explicitly False
values, allowing both connection extras and parameter overrides to work correctly.

* Fix mypy no-redef errors by removing duplicate type annotations

* Fix import order for static checks

* Trigger CI re-run
itayweb pushed a commit to itayweb/airflow that referenced this pull request Dec 6, 2025
…as (apache#58348)

* Fix Google provider to handle False boolean values in connection extras

Fixed a bug where boolean fields set to False in Google provider
connection extras were incorrectly returning default values instead.

The issue occurred in two places:
1. get_field() function used 'or None' which treated False as falsy
2. _get_field() method used compound boolean logic that returned
   the default value when the actual value was False

This particularly affected BigQueryHook where use_legacy_sql=False
in connection extras would incorrectly return True.

Changes:
- Modified get_field() to only treat empty strings as None
- Modified _get_field() to explicitly check 'is not None'
- Added tests for False/falsy value handling

Fixes apache#47053

* Fix test import paths for Airflow 3.x compatibility

Update mock paths from 'airflow.hooks.base.BaseHook' to
'airflow.providers.common.compat.sdk.BaseHook' to match
the new import structure in Airflow 3.x.

* Fix Google provider to respect False boolean values in connection extras

Use sentinel pattern to distinguish unset parameters from explicitly False
values, allowing both connection extras and parameter overrides to work correctly.

* Fix mypy no-redef errors by removing duplicate type annotations

* Fix import order for static checks

* Trigger CI re-run
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When the field of providers-google is False, the value is incorrect

3 participants