Virtual field name cannot be used in strings
Open, Needs TriagePublicBUG REPORT
Actions

Assigned To

None

Authored By

	HertzDevil
	Sep 6 2019, 9:45 AM

Project Tags

Referenced Files

None

Subscribers

Description

Tested on MW 1.31.2 and Cargo 2.2. If a table contains a virtual field, the name of that field cannot be used inside strings of query parameters. For example, given the table

{{#cargo_declare:_table=Bug
|abc=String
|xyz=List (,) of Integer
}}

the query

{{#cargo_query:tables=Bug|where=abc='xyz'}}

fails with this message:

Error: operator for the virtual field 'Bug.xyz' must be 'HOLDS', 'HOLDS NOT', 'HOLDS LIKE' or 'HOLDS NOT LIKE'.

Similarly, in Lua the following query also results in the same error:

mw.ext.cargo.query('Bug', '_pageName', {where = "abc='xyz'"})

This particular error can be avoided by using abc=CONCAT('x','yz') as the where-clause; since xyz does not appear literally in the clause, Cargo runs the query fine without errors. In general, however, the where-clause may be assembled from template or module parameters, in which case every query needs to be manually escaped in such a contrived manner. Live example.

Event Timeline

HertzDevil created this task.Sep 6 2019, 9:45 AM

Restricted Application added subscribers: Liuxinyu970226, Aklapper. · View Herald TranscriptSep 6 2019, 9:45 AM

Paul.Troughton subscribed.Nov 20 2022, 11:46 AM

I have also run into this issue, in Cargo 3.1. It looks like it's still present in HEAD, as the relevant bits of code haven't changed.

In our use case, the contents of the where clause are dynamic, and only sometimes happen to include a virtual field name. This makes it hard to use HertzDevil's workaround.

The root cause is that the regexp used on line 947 of includes/CargoSQLQuery.php to trigger this exception is insufficiently restrictive, such that it matches any virtual field name anywhere in the where clause. The regexp is generated by CargoUtils::getSQLTableAndFieldPattern, and takes the form /([^\w$.,]|^)VIRTUALFIELDNAME([^\w$]|$)/.

Unfortunately, my regexp skills aren't up to coming up with an improved expression which will match VIRTUALFIELDNAME where it's being used as a field name but not when it's just part of a string value. So in the meantime, we have just commented out the lines that throw this exception (947-956 of CargoSQLQuery.php).

I recently ran into a similar issue when using the table alias inside strings as well, in Cargo 3.2

In my case I don't get any error, the query just doesn't return any result although it should

{{#cargo_query:
tables=Table=T
|fields=T.TestField
|where=T.TestField = "T.Whatever"
}}

My temporary fix was to change the table alias to something else, in my case Table=To, but obviously if a query gets executed using "To.<whatever>" it will have the same issue

Yaron_Koren moved this task from Backlog to Queries on the MediaWiki-extensions-Cargo board.Jun 1 2023, 8:00 PM

This is the same problem as encountered here:

I suggested a patch on the latter link:

Here is the patch. In includes/CargoSQLQuery.php at line 987 replace

if ( preg_match( $patternSimple[$i], $this->mWhereStr ) ) {

with this:

# Don't want to risk matching a value that is also the field name
# So remove value from comparison string 
if (preg_match('~(?:`_value`|_pageName)\s*=\s*[\'"](.*?)[\'"]~', $this->mWhereStr, $matches) ){
	$temp = str_replace($matches[1], '', $this->mWhereStr);
}
if ( preg_match( $patternSimple[$i], $temp ) ) {

I've been using that for a year or so with no known problems.

Jonathan3 added a project: Patch-For-Review.Jul 8 2026, 12:13 AM

@Yaron_Koren My new friend Claude recommended this instead:

--- a/includes/CargoSQLQuery.php
+++ b/includes/CargoSQLQuery.php
@@ -987,7 +987,10 @@ class CargoSQLQuery {
                                                $fieldReplaced
                                        );

-                                       if ( preg_match( $patternSimple[$i], $this->mWhereStr ) ) {
+                                       # Was: if ( preg_match( $patternSimple[$i], $this->mWhereStr ) ) {
+                                       # Don't want to risk matching a value that is also a field name
+                                       $temp = CargoUtils::removeQuotedStrings( $this->mWhereStr );
+                                       if ( preg_match( $patternSimple[$i], $temp ) ) {
                                                if ( $isHierarchy ) {
                                                        throw new MWException( "Error: operator for the hierarchy field '" .
                                                                "$tableName.$fieldName' must be 'HOLDS', 'HOLDS NOT', '" .

Virtual field name cannot be used in stringsOpen, Needs TriagePublicBUG REPORTActions

Description

Event Timeline

Virtual field name cannot be used in strings
Open, Needs TriagePublicBUG REPORT
Actions