Skip to content

Conversation

@psharma-1909
Copy link

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds database feature checks and modifies test models to support SingleStore database compatibility. The changes ensure tests requiring database defaults (db_default) and other specific features only run when supported by the database backend.

  • Added @skipUnlessDBFeature("supports_db_default") decorators to tests using db_default functionality
  • Modified test models to use explicit through tables for ManyToMany relationships and added SingleStore-specific ModelStorageManager
  • Simplified field default models to use static values instead of database function expressions
  • Adjusted test expectations for query counts and serialized output
  • Fixed test field references in test_prepare_join_on_clause

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
tests/validation/test_unique.py Added feature check for supports_db_default to test_unique_db_default test
tests/schema/tests.py Added feature checks for supports_db_default to multiple database default tests
tests/model_inheritance/models.py Added ModelStorageManager to multiple inheritance models for SingleStore compatibility
tests/model_formsets/test_uuid.py Added explicit save operations for parent and formset
tests/migrations/test_operations.py Added feature checks and corrected decorator from supports_expression_defaults to supports_db_default
tests/force_insert_update/models.py Added import and ModelStorageManager for inheritance models
tests/fixtures/tests.py Updated expected query count and removed permissions field from serialized output
tests/field_defaults/tests.py Added debug output and changed test assertions to use static values instead of dynamic functions
tests/field_defaults/models.py Simplified DBDefaultsFunction model to use static values instead of database functions
tests/constraints/tests.py Added feature check for supports_db_default to constraint test
tests/basic/tests.py Added feature check for supports_db_default to primary key test
tests/backends/models.py Added explicit through table for SchoolBus ManyToMany relationship
tests/backends/base/test_operations.py Changed test to use author_name_field instead of author_id_field

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 55 to 83
print("\n=== DEBUGGING SQL GENERATION ===")
schema_editor = connection.schema_editor()

# Check how defaults are generated
for field in DBArticle._meta.fields:
if hasattr(field, 'db_default') and field.db_default is not None:
try:
default_sql, params = schema_editor.db_default_sql(field)
print(f"Field '{field.name}' default SQL: {default_sql}")
except Exception as e:
print(f"Field '{field.name}' error: {e}")

# Check actual table structure
with connection.cursor() as cursor:
cursor.execute("DESCRIBE field_defaults_dbarticle")
columns = cursor.fetchall()
print("\nActual table columns:")
for col in columns:
print(f" {col}")

# Now run the actual test
a = DBArticle()
a.save()
a.refresh_from_db()

print(f"\nActual values after refresh:")
print(f" headline: {a.headline}")
print(f" pub_date: {a.pub_date}")
print(f" cost: {a.cost}")
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

Debug print statements should be removed from production test code. These debugging statements appear to have been accidentally left in the test after troubleshooting.

Copilot uses AI. Check for mistakes.
Comment on lines 90 to 106
# === DEBUGGING SQL GENERATION ===
# Field 'id' error: type object 'NOT_PROVIDED' has no attribute 'as_sql'
# Field 'headline' default SQL: 'Default headline'
# Field 'pub_date' default SQL: (CURRENT_TIMESTAMP(6))
# Field 'cost' default SQL: 3.33

# Actual table columns:
# ('id', 'bigint(20)', 'NO', 'PRI', None, 'auto_increment')
# ('headline', 'varchar(100)', 'NO', '', 'Default headline', '')
# ('pub_date', 'datetime(6)', 'NO', '', 'Now()', '')
# ('cost', 'decimal(3,2)', 'NO', '', '3.33', '')

# Actual values after refresh:
# headline: Default headline
# pub_date: None
# cost: 3.33

Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

Commented-out debug output should be removed from production test code. This appears to be leftover debugging information that should not be committed.

Suggested change
# === DEBUGGING SQL GENERATION ===
# Field 'id' error: type object 'NOT_PROVIDED' has no attribute 'as_sql'
# Field 'headline' default SQL: 'Default headline'
# Field 'pub_date' default SQL: (CURRENT_TIMESTAMP(6))
# Field 'cost' default SQL: 3.33
# Actual table columns:
# ('id', 'bigint(20)', 'NO', 'PRI', None, 'auto_increment')
# ('headline', 'varchar(100)', 'NO', '', 'Default headline', '')
# ('pub_date', 'datetime(6)', 'NO', '', 'Now()', '')
# ('cost', 'decimal(3,2)', 'NO', '', '3.33', '')
# Actual values after refresh:
# headline: Default headline
# pub_date: None
# cost: 3.33

Copilot uses AI. Check for mistakes.
}
)
self.assertIs(formset.is_valid(), True)

Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

Trailing whitespace should be removed for code cleanliness.

Suggested change

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@psharma-1909 please apply suggestion


# Save parent first
formset.instance.save()

Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

Trailing whitespace should be removed for code cleanliness.

Suggested change

Copilot uses AI. Check for mistakes.

# Then save the formset
formset.save()

Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

Trailing whitespace should be removed for code cleanliness.

Suggested change

Copilot uses AI. Check for mistakes.
@psharma-1909 psharma-1909 requested a review from Copilot October 31, 2025 09:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 53 to 54
def test_field_db_defaults_refresh(self):

Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The function definition line 53 has trailing whitespace, and line 54 is an empty line with extra whitespace. Remove trailing spaces.

Suggested change
def test_field_db_defaults_refresh(self):
def test_field_db_defaults_refresh(self):

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@psharma-1909 please apply suggestion

a = DBArticle()
a.save()
a.refresh_from_db()

Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Line 58 has trailing whitespace after the blank line. Remove trailing spaces.

Suggested change

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@psharma-1909 please apply suggestion

a = DBArticle()
a.save()
a.refresh_from_db()

Copy link
Collaborator

Choose a reason for hiding this comment

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

@psharma-1909 please apply suggestion

}
)
self.assertIs(formset.is_valid(), True)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@psharma-1909 please apply suggestion

@psharma-1909 psharma-1909 merged commit 651ce5a into singlestore-test-5.0.x Nov 6, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants