Skip to content

No static typing#5673

Open
line-o wants to merge 3 commits intoeXist-db:developfrom
line-o:no-static-typing
Open

No static typing#5673
line-o wants to merge 3 commits intoeXist-db:developfrom
line-o:no-static-typing

Conversation

@line-o
Copy link
Member

@line-o line-o commented Mar 16, 2025 •
edited
Loading

Description:

fixes #2445

Reference:

Type of tests:

XQSuite tests

@line-o line-o requested a review from a team as a code owner March 16, 2025 19:11
dizzzz
dizzzz requested changes Mar 16, 2025
View reviewed changes
Copy link
Member

@dizzzz dizzzz left a comment

Choose a reason for hiding this comment

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

Build is failing;
can we add tests eg from the original bug report?

@dizzzz dizzzz requested review from a team and wolfgangmm March 16, 2025 20:02
@line-o line-o force-pushed the no-static-typing branch 2 times, most recently from 06f4eea to a12a11e Compare March 16, 2025 20:56
@line-o
Copy link
Member Author

line-o commented Mar 16, 2025

I applied commits from an older PR that targeted develop-6.x.x and it took 3 attempts to get it to compile.
I will look to add one or two tests from the original issue.

@line-o
Copy link
Member Author

line-o commented Mar 16, 2025

I would expect for one to many additional XQTS test cases to now pass.

@line-o
Copy link
Member Author

line-o commented Mar 16, 2025

There seems to be zero tests that assert error XPST0005 to be raised. ;)

@dizzzz
Copy link
Member

dizzzz commented Mar 17, 2025

I am wondering.... should this code change be 'protected' to it would not be effective for 'query 3.1' (and older) based queries?

@line-o
Copy link
Member Author

line-o commented Mar 17, 2025 •
edited
Loading

The error with code XPST0005 should only be raised when a XQuery processor implements the static typing feature.

exist-db never claimed to support this optional feature, which is why I believe we do not need to guard against it.
I do agree that one can argue it is a breaking change which is why I ported my original PR #3653 to target exist 7 instead.

With this PR applied XPath queries will behave much more like in any other Xquery processor (namely baseX and Saxon).

@line-o line-o force-pushed the no-static-typing branch from a12a11e to 48082b2 Compare March 17, 2025 18:18
@line-o line-o mentioned this pull request Mar 17, 2025
Closed
@line-o line-o force-pushed the no-static-typing branch 3 times, most recently from 773a278 to 51e4984 Compare March 17, 2025 20:10
@line-o line-o requested a review from dizzzz March 17, 2025 21:42
dizzzz
dizzzz approved these changes Mar 18, 2025
View reviewed changes
Copy link
Member

@dizzzz dizzzz left a comment

Choose a reason for hiding this comment

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

as discussed in telco

@dizzzz dizzzz requested review from a team and reinhapa March 20, 2025 16:10
@dizzzz dizzzz requested a review from a team March 24, 2025 10:28
duncdrum
duncdrum approved these changes Mar 24, 2025
View reviewed changes
Copy link
Contributor

@duncdrum duncdrum left a comment

Choose a reason for hiding this comment

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

as discussed needs documentation

@line-o line-o added the needs documentation Signals issues or PRs that will require an update to the documentation repo label Apr 15, 2025
@line-o line-o force-pushed the no-static-typing branch from 51e4984 to d7d13a2 Compare July 30, 2025 21:15
@adamretter
Copy link
Contributor

adamretter commented Aug 4, 2025 •
edited
Loading

An interesting idea of how to fix the issue... But it must be recognised that this small change is very significant!

Before it is merged two things need to be checked carefully in great detail:

  1. No regression in the XQTS results
  2. No performance regression (looking at the code change, I suspect this PR will result in a regression in its current form)

adamretter
adamretter suggested changes Aug 4, 2025
View reviewed changes
Copy link
Contributor

@adamretter adamretter left a comment

Choose a reason for hiding this comment

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

I would like to see a full report of the before-and-after XQTS results, and a number of performance tests added that should show this does not lead to a performance regression.

@line-o line-o mentioned this pull request Dec 9, 2025
Merged
@line-o
[refactor] ContextItemExpression uses staticReturnType
bcb062c 
Turns out, a property for the static return type is already defined by
the Step super class.
ContextItemExpression uses this now and its own returnType property is
removed.
line-o and others added 2 commits December 18, 2025 10:48
@line-o
[fix] simple predicates evaluating to empty sequences
545513c 
Fixes eXist-db#2445

By definition the following Xpaths must evaluate to an empty sequence
- `<a/>[self::b]`
- `<a/>/@b/c`
- `<a/>/text()/b`

Instead of throwing an Exception the staticReturnType of the current step
and the contextInfo returnType are set to Type.EMPTY.
@line-o
[test] add tests to ensure no static typing
ee6299f 
refs eXist-db#2445
These tests aim to ensure that certain XPath expressions no longer raise error XPST0005 as exist-db does not implement
the static typing feature and this feature is even dropped entirely with the next version of XQuery.
@line-o
Copy link
Member Author

line-o commented Feb 16, 2026

I see a minor - but consistent - performance improvement.

Execution times of the entire test suite goes down from 693.4 seconds to 667.3 in the last XQTS run in CI. That is 3.75% faster.
I observed similar results on my local machine.

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

Reviewers

@dizzzz dizzzz dizzzz approved these changes

@reinhapa reinhapa reinhapa approved these changes

@duncdrum duncdrum duncdrum approved these changes

@wolfgangmm wolfgangmm Awaiting requested review from wolfgangmm

+1 more reviewer

@adamretter adamretter adamretter requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs documentation Signals issues or PRs that will require an update to the documentation repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

self:: axis inside predicate throws XPST0005

5 participants

@line-o @dizzzz @adamretter @reinhapa @duncdrum