QOQ distinct with Datetime column

psicard · July 29, 2024, 4:56pm

After an upgrade to Lucee 6, we get a strange result if we are using a QOQ with a distinct to a datetime field.

I been able to reproduce it with that code

var rsData = queryNew( "scheduleLabel,scheduleTime", "varchar,datetime");
for(var i=1; i<= 50; i++) {
	var scheduleTime  = createDateTime(2024, 12, 12, i mod 12, 0, 0,0)
	rsData.addRow({scheduleLabel: "Event : #i mod 12#", scheduleTime: scheduleTime});
}

var rsUniqueDataTime = queryExecute(
    sql = "SELECT DISTINCT scheduleTime FROM rsData ORDER BY scheduleTime",
    options = {
        dbtype = 'query'
    }
);
 
writeDump(var=rsData, label="rsData");
writeDump(var=rsUniqueDataTime, label="rsUniqueDataTime");

As you can see, the distinct don’t return the result expected

OS: Windows 10 (10.0) 64bit
Servlet Container WildFly / Undertow - 2.2.24.Final
Java Version: 11.0.19 (Eclipse Adoptium) 64bit
Lucee Version: 6.0.3.1

TonyMonast · July 29, 2024, 5:49pm

Hi,

To narrow down the problem, does it do the same thing using GROUP BY instead of DISTINCT?

psicard · July 29, 2024, 6:01pm

Yes I have a result similar as you can see from the screenshot. The result is random too. I presume that caused by the parallelism put in place under Lucee 6.

Roberto_Marzialetti · August 2, 2024, 7:48pm

Here seems works fine.
I’m using Lucee 6.0.0+SNAPSHOT+451 (by TryCF)

dump(now());

var q = QueryNew("label,thisDate", "varchar,datetime");

cfloop( from=1, to=20, index="index") {

    var addValue = 10;

    if ( index > 7 AND index < 15 ) {
        var addValue = 20;
    }

    if ( index > 15 ) {
        var addValue = 50;
    }

    QueryAddRow( 
        q, 
        { thisDate: DateAdd( "n", addValue, now()), label: "index: #index# " }
    );

}


<cfdump var="#q#">

<cfquery dbtype="query" name="j">
    SELECT distinct thisDate
    FROM q 
    ORDER BY thisDate
</cfquery>

<cfdump var="#j#">

psicard · August 7, 2024, 6:36pm

I try your code from my side. You are right that not causing issue but if I use more rows. ex: 200 instead of your 20 rows. I got that result.

Based on Improving Lucee's QoQ Support Again- now 200% faster

By default, any QoQ on a query object less than 50 rows will execute sequentially (no threads) because the overhead of managing the joining the threads is normally more than the benifit. 50+ rows seems to be where the benifit outweighs the overhead, so all query objects with that least that many rows will be processed in parallel.

I have maybe the impression that could be related to it.

If I set the lucee variable lucee.qoq.parallelism=9999999. I don’t have the issue anymore.

psicard · August 22, 2024, 5:52pm

@Brad_Wood Do you think the issue could be related to the change made for the improvement of QoQ?

bdw429s · August 22, 2024, 6:55pm

Interesting. Does this ONLY happen with datetime columns, or any type?

The partitioning SHOULD be thread safe. The grouped data is stored in a current hashmap (thread safe)

github.com

lucee/Lucee/blob/423fd0093c2fb44c7a5ab9990b996c7c6301d2c5/core/src/main/java/lucee/runtime/sql/QueryPartitions.java#L60


      
          	// Array of keys for fast lookup
          	private Collection.Key[] columnKeys;
          	// Needed for functions and aggregates but not explicitly part of the final select
          	private Set<Collection.Key> additionalColumns;
          	// Group by expressions
          	private Expression[] groupbys;
          	// Target query for column references
          	private QueryImpl target;
          	// Mapof partitioned query data. Key is unique string representing grouped data, value is a
          	// Query object representing the matching rows in that group/partition
          	private ConcurrentHashMap<String, QueryImpl> partitions = new ConcurrentHashMap<String, QueryImpl>();
          	// Reference to QoQ instance
          	private QoQ qoQ;
          	// SQL instance
          	private SQL sql;
          
          	/**
          	 * Constructor
          	 *
          	 * @param sql
          	 * @param columns

and the logic to add a new row uses computeIfAbsent() which is advertised as an atomic operation, ensuring internal locking as necessary.

github.com

lucee/Lucee/blob/423fd0093c2fb44c7a5ab9990b996c7c6301d2c5/core/src/main/java/lucee/runtime/sql/QueryPartitions.java#L133


      
          	 * @param source Source query to get data from
          	 * @param row Row to get data from
          	 * @param finalizedColumnVals If we're adding finalized data, just copy it across. Easy. This
          	 *            applies when distincting a result set after it's already been processed
          	 * @throws PageException
          	 */
          	public void addRow(PageContext pc, QueryImpl source, int row, boolean finalizedColumnVals) throws PageException {
          		// Generate unique key based on row data
          		String partitionKey = buildPartitionKey(pc, source, row, finalizedColumnVals);
          		// Create partition if necessary
          		QueryImpl targetPartition = partitions.computeIfAbsent(partitionKey, k -> {
          			try {
          				return createPartition(target, source, finalizedColumnVals);
          			} catch( Exception e ) {
          				throw new RuntimeException( e );
          			}
          		} );
          
          		int newRow = targetPartition.addRow();
          
          		// If we're adding finalized data, just copy it across. Easy. This applies when distincting

Is it possible your dates have a millisecond component to them which is not reflected in your output, but makes then not equal to each other?

bdw429s · August 22, 2024, 7:02pm

I can’t seem to reproduce this on trycf (running Lucee 6.1.0.243) even with 1000 rows in the query

psicard · August 22, 2024, 7:15pm

I try with different type. I never been able to reproduce it with an other type. I even change my original query and cast it as a varchar and that was resolving the issue too.

My colleague had the same idea. We valid it the millisecond are equal. The result can be random even if the query return the same values and have an order on a Primary key.

bdw429s · August 22, 2024, 7:33pm

I wonder if the algorithm which creates the unique key for each row is serializing the dates differently. It involves the to string casting and, if the value is longer than 255 chars, an MD5 hash. I don’t think the hash would kick in for a date, but it’s possible the string caster isn’t giving back the same value for each one. I need to be able to reproduce it though, and as I posted above, it’s not reproducing for me on trycf.

psicard · August 22, 2024, 8:28pm

You just give me an hint with your MD5. I been able to reproduce it under your trycf by adding at the begin these 2 lines

settimezone(“Europe/Paris”);
SetLocale(“French (Standard)”);

I presume the cast of the datetime is different depending of the timezone/locale ?

bdw429s · August 22, 2024, 9:11pm

Wow, nice find! I assume there’s something related with the string caster casting dates with a time zone that maybe isn’t thread safe. I haven’t been able to pinpoint that just yet. I’m not 100% sure which code path is being followed to convert the dates into strings for the partition keys behind the scenes.