Skip to content

Schema lookup implementation#9

Open
rishipradeep-think41 wants to merge 4 commits into
think41:devfrom
rishipradeep-think41:main
Open

Schema lookup implementation#9
rishipradeep-think41 wants to merge 4 commits into
think41:devfrom
rishipradeep-think41:main

Conversation

@rishipradeep-think41
Copy link
Copy Markdown

Summary

This PR implements support for schema lookup directly from the connected database when generating SQL queries, addressing think41/foundation-sql#8. Previously, a schema.sql file or a hardcoded schema string was required for schema understanding. With this change, the system can now retrieve schema information dynamically from the database.

Related Issue

Closes #8

Changes

  • Added logic to fetch schema details from the connected database if schema.sql or schema is not present.
  • Minor changes in master prompt to try and improve SQL generation.
  • Changes in query.py and common.py to support all 3 modes of usage (schema, file, and fetch schema from DB)
  • Added a new test case to test the functionality of schema lookup
  • Added a util.py file to create and manage a test db for above test case

Additional Notes

  • Let me know if further tests or documentation updates are needed.

Comment thread foundation_sql/db.py
# Check the response type and transform accordingly
if model_class == int:
return int(unflattened_data["result"])
# FIX : STILL ONLY GETS FIRST LINE OF RESPONSE
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We can be a bit more defensive here i.e. do next / iter only if it is a list / iterator.

Comment thread tests/utils.py Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Probably not needed. It is test specific and can be in test_schema_discovery.

Comment thread tests/test_schema_discovery.py Outdated

create_bike_db()

query = common.create_query(db_url=f"sqlite:///{BIKES_DB_PATH}")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be better to give schema discovery as an option i.e. set it as following parameters

schema=None, schema_file=None, schema_inspect=False

If schema_inspect is true, none of the other two should be passed or it raises an error. Otherwise, one of schema_file or schema string should be passed.

Gives more flexibility to the developer (user) of the library that way.

Comment thread tests/common.py Outdated
"""Base test class for database-driven tests with common setup and helper methods."""

db_url = DB_URL
db_url = "sqlite:///:memory:"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why the hardcoding?

Comment thread foundation_sql/query.py
self.system_prompt = system_prompt or self.load_file(system_prompt_path)
else:
self.system_prompt = DEFAULT_SYSTEM_PROMPT

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why this removal?

Comment thread .gitignore Outdated

__sql__ No newline at end of file
__sql__
fixtures No newline at end of file
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why this addition of fixtures?

Copy link
Copy Markdown
Collaborator

@anshum4n-git anshum4n-git left a comment

Choose a reason for hiding this comment

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

Looks like I missed submitting this review. Take a look at the comments. Also, if we can separate out this PR and #10 for Multiple database support, they can be merged separately.

@anshum4n-git anshum4n-git changed the base branch from main to dev September 2, 2025 04:52
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.

Schema Lookup support

3 participants