Skip to content

Conversation

@samialfattani
Copy link
Contributor

This PR address the issue that raises whenever the end user navigate to any page using MSSQL database. The MSSQL restricts using LIMIT and OFFSET without sorting,

  • Some test cases are added using the paginations with single and muliple primary keys.
  • The MSSQL docker container is added on port 1433.

@samialfattani samialfattani changed the title MSSQL MSSQL Compatibility Dec 20, 2025
@samialfattani samialfattani marked this pull request as ready for review December 20, 2025 19:06
@samialfattani
Copy link
Contributor Author

tests are failing due to the missing of database conneciton with MSSQL, knowing that it suppposed to configured with devcontainer . can any one help and guide me how to activate the MSSQL divcontainer during the test ??

@ElLorans
Copy link
Contributor

I silenced the warning in master, can you pull the latest changes?

@ElLorans
Copy link
Contributor

Thanks for the great work! I do not see any change to functionality, so what was the issue?

@samialfattani
Copy link
Contributor Author

Thanks for the great work! I do not see any change to functionality, so what was the issue?

to replicate the error:

1- Create a ModelView that connected to MSSQL Table includes more number of records to be paginated.
2- once the ModelView has no default sorting, the MSSQL will reject using LIMIT and OFFSET without ORDER BY clause.

@ElLorans
Copy link
Contributor

But I cannot see where you fixed the issue. Sorry for being dense.

@samialfattani
Copy link
Contributor Author

i can't see this either 😄 , honestly this PR gone throught many branches and local merges until i forget what i've done , maybe the issue is just addressed throught all perviouse PRs. Anyway, we can just consider this PR as enabling the use of MSSQL with a very limited test cases. later on, i will modify the /tests/sqla ALL functions to be parametrized so that they should be tested againest (postgres, mssql , and sqllite)

@ElLorans
Copy link
Contributor

Is pymssql required? I do not see any import.
Since the tests are currently passing, I think we can directly remove this PR and parametrize the main db fixture.

@samialfattani
Copy link
Contributor Author

samialfattani commented Dec 26, 2025

Is pymssql required? I do not see any import.

Yes it is, SQLAlchmey impots it internally. without this PR MSSQL test won't pass since no container of MSSQL. this PR added container configuration for both devcontainer and github/workflow

@ElLorans
Copy link
Contributor

I would then convert/close this PR to parametrize the db fixture, since the additional test can removed.

@samialfattani
Copy link
Contributor Author

yes sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants