You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When an R UDF is defined with an output using a date type field as an output argument of an embedded R UDF the field fails with a message "Failed to convert column 0"
Reproducible: Always
Steps to Reproduce:
1.Create a table with dates
CREATE TABLE testTableDates (d DATE);
INSERT INTO testTableDates(d) VALUES ('2019-01-01'),('2019-01-02'),('2019-01-03');
Create a simple loopback R based UDF
CREATE FUNCTION testDateLoopback(din DATE) RETURNS TABLE (dout DATE) LANGUAGE R {
data.frame(dout=din)
}
Call loopback the UDF
SELECT * FROM testDateLoopback( (SELECT * FROM testTableDates) )
Actual Results:
"Failed to convert column 0"
Expected Results:
'2019-01-01'
'2019-01-02'
'2019-01-03'
Dug into the code base and found that there is a TODO for this but cannot find any ticket raised for it:
bat_to_sexp: Works sufficiently due to bat type being an integer although R sees the date as an integer.
sexp_to_bat: Looks at the UDF arg type and fails to convert as no TYPE_Date defined
I have created an initial fix for this ready for review but cannot find any details about how to access the repository in order to push these changes and contribute to the project. Please advise on how to proceed.
You can prepare a patch (preferably with the command hg export) and add it as an attachment to this issue so that someone can review it. If you need more information let us know.
I've attached an initial patch for review. I've fixed the date output and also the representation within R.
I was tempted to refactor some code in mtime as I noticed it defines a date_isnil but other types are in the format id_{type}_nil. I've created an alias define in this patch. Can we refactor mtime or is there a particular reason for this inconsistent naming?
Also, I'd like to add some tests for the changes but not sure how best to write them in the bat format.
Is there a quick way to generate the following in bat format?
//Proves class is Date rather than Numeric
CREATE FUNCTION getClassName(din DATE) RETURNS TABLE (dout TEXT) LANGUAGE R {
data.frame(dout=class(din))
}
SELECT * FROM getClassName( (SELECT * FROM T001) )
//Prove that conversion is correctly offsetting for epoch
CREATE FUNCTION passthroughConversionCheck(din DATE) RETURNS TABLE (dout DATE) LANGUAGE R {
data.frame(dout=as.Date(as.double(din), origin="1970-1-1") )
}
SELECT * FROM passthroughConversionCheck( (SELECT * FROM T001) )
Thanks for the patch. Part of our team is in the middle of moving to a new building so this might take some time to process.
About your question with respect to mtime, revisiting this code has been one of our TODO items for some time now, but as far as I know there has been a number of related issues that we would like to visit at the same time. I will check and get back to you as soon as I can.
Thank you again,
Panos.
Comment 26792
Date: 2019-01-14 16:15:53 +0100
From: Paul <>
Thanks for the update Panos, good luck with the move.
Useful to know that about mtime, I figured as much given the almost "here be dragons" style comments and the well documented history of time in there, fun read!
That reminds me I was doing further testing and can't quite get my head around one of the results.
A date defined as null returns as null with a direct query, but when it involves a call to an R UDF it returns as nil. I thought I had used the int null check rather than dbl null check but switching those makes no difference difference. When you have a moment I'd love to get your thoughts on where that comes from - I wasn't aware nil was a valid value to store.
I was tempted to refactor some code in mtime as I noticed it defines a
date_isnil but other types are in the format id_{type}_nil. I've created an
alias define in this patch. Can we refactor mtime or is there a particular
reason for this inconsistent naming?
Historical reasons.
I've now changed the macros to is_date_nil etc.
Date: 2019-01-06 14:47:26 +0100
From: Paul <>
To: MonetDB5 devs <>
Version: 11.31.11 (Aug2018-SP1)
CC: @kutsurak
Last updated: 2019-04-30 12:36:04 +0200
Comment 26779
Date: 2019-01-06 14:47:26 +0100
From: Paul <>
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0
Build Identifier:
When an R UDF is defined with an output using a date type field as an output argument of an embedded R UDF the field fails with a message "Failed to convert column 0"
Reproducible: Always
Steps to Reproduce:
1.Create a table with dates
SELECT * FROM testDateLoopback( (SELECT * FROM testTableDates) )
Actual Results:
"Failed to convert column 0"
Expected Results:
'2019-01-01'
'2019-01-02'
'2019-01-03'
Dug into the code base and found that there is a TODO for this but cannot find any ticket raised for it:
bat_to_sexp: Works sufficiently due to bat type being an integer although R sees the date as an integer.
sexp_to_bat: Looks at the UDF arg type and fails to convert as no TYPE_Date defined
I have created an initial fix for this ready for review but cannot find any details about how to access the repository in order to push these changes and contribute to the project. Please advise on how to proceed.
Comment 26780
Date: 2019-01-07 17:29:09 +0100
From: @kutsurak
Hello Paul,
The official MonetDB mercurial repository is hosted at
https://dev.monetdb.org/hg/MonetDB/
You can prepare a patch (preferably with the command hg export) and add it as an attachment to this issue so that someone can review it. If you need more information let us know.
Best regards,
Panos.
Comment 26784
Date: 2019-01-09 13:28:52 +0100
From: Paul <>
Created attachment 613
initial patch for review
Comment 26785
Date: 2019-01-09 13:29:15 +0100
From: Paul <>
Thanks Panos,
I've attached an initial patch for review. I've fixed the date output and also the representation within R.
I was tempted to refactor some code in mtime as I noticed it defines a date_isnil but other types are in the format id_{type}_nil. I've created an alias define in this patch. Can we refactor mtime or is there a particular reason for this inconsistent naming?
Also, I'd like to add some tests for the changes but not sure how best to write them in the bat format.
Is there a quick way to generate the following in bat format?
//Proves class is Date rather than Numeric
CREATE FUNCTION getClassName(din DATE) RETURNS TABLE (dout TEXT) LANGUAGE R {
data.frame(dout=class(din))
}
SELECT * FROM getClassName( (SELECT * FROM T001) )
//Prove that conversion is correctly offsetting for epoch
CREATE FUNCTION passthroughConversionCheck(din DATE) RETURNS TABLE (dout DATE) LANGUAGE R {
data.frame(dout=as.Date(as.double(din), origin="1970-1-1") )
}
SELECT * FROM passthroughConversionCheck( (SELECT * FROM T001) )
Comment 26791
Date: 2019-01-14 16:00:16 +0100
From: @kutsurak
Hi Paul,
Thanks for the patch. Part of our team is in the middle of moving to a new building so this might take some time to process.
About your question with respect to mtime, revisiting this code has been one of our TODO items for some time now, but as far as I know there has been a number of related issues that we would like to visit at the same time. I will check and get back to you as soon as I can.
Thank you again,
Panos.
Comment 26792
Date: 2019-01-14 16:15:53 +0100
From: Paul <>
Thanks for the update Panos, good luck with the move.
Useful to know that about mtime, I figured as much given the almost "here be dragons" style comments and the well documented history of time in there, fun read!
That reminds me I was doing further testing and can't quite get my head around one of the results.
A date defined as null returns as null with a direct query, but when it involves a call to an R UDF it returns as nil. I thought I had used the int null check rather than dbl null check but switching those makes no difference difference. When you have a moment I'd love to get your thoughts on where that comes from - I wasn't aware nil was a valid value to store.
Comment 26873
Date: 2019-02-04 15:37:33 +0100
From: MonetDB Mercurial Repository <>
Changeset fe2012cbd8dc made by Sjoerd Mullender sjoerd@acm.org in the MonetDB repo, refers to this bug.
For complete details, see https//devmonetdborg/hg/MonetDB?cmd=changeset;node=fe2012cbd8dc
Changeset description:
Comment 26874
Date: 2019-02-04 17:35:49 +0100
From: @sjoerdmullender
(In reply to Paul from comment 3)
Historical reasons.
I've now changed the macros to is_date_nil etc.
Comment 26932
Date: 2019-03-19 09:29:10 +0100
From: MonetDB Mercurial Repository <>
Changeset ee38b509dbe9 made by Sjoerd Mullender sjoerd@acm.org in the MonetDB repo, refers to this bug.
For complete details, see https//devmonetdborg/hg/MonetDB?cmd=changeset;node=ee38b509dbe9
Changeset description:
Comment 26933
Date: 2019-03-19 09:30:18 +0100
From: @sjoerdmullender
Patch applied, test added. Closing.
The text was updated successfully, but these errors were encountered: