SQlite data source for RDataFrame #2322

jblomer · 2018-07-16T07:28:10Z

This PR adds a new data source for RDataFrame that is able to provide data from SQlite SELECT queries. It will be useful for cvmfs, where we have file catalogs and monitoring information in sqlite files. For instance, one can do

auto rdf = ROOT::RDF::MakeSqliteDataFrame("catalog.sqlite", "select * from catalog");
auto h = rdf.Define("lname", "name.length()").Histo1D("lname");

to show the distribution of file name sizes. There are probably more use cases, for instance sqlite export of conditions data.

This is work in progress, I'm posting it for early comments and feedback.

My open points are

Unit and integration tests are yet to be done
The data source work single-threaded only at the moment. I initially thought it might be enough to return only a single row in GetEntryRanges() to make it thread-safe but that's apparently not enough. So I'm now thinking to wrap SetEntry() and GetEntryRanges() in a mutex.
The code is not reusing TSQLiteServer. It felt like it requires stretching the TSQLServer interface in perhaps unwanted ways, for saving only a handful of direct sqlite API calls. On the other hand, with (probably quite a bit) more work it might be possible to have a more general data source that works on any SQL result set.
Determining column types in SQlite is tricky as it is dynamically typed and in principle each row can have different column types. If a table column is queried as is, I can use the default/declared column type. For expressions, I'll use the type of the first row of the result set. Still it can result in a column to be of type NULL where subsequent rows actually have meaningful values. My guess is that the current heuristic is practical enough, and of course the user can formulate the SELECT query to avoid ambiguity.
It would not be impossible to add support for remote reading of sqlite files. To do so, one can add a custom implementation of an sqlite virtual file system to have data pouring in directly from HTTP or XRootD. This is perhaps something to keep in mind.

jblomer · 2018-07-20T13:19:15Z

@dpiparo @bluehood From my side this is ready for review. I think it makes sense to have remote reading of files in a possible follow-up PR. Please let me know if you think it makes sense to add an integration test to roottest. In this case, how would I deal with the fact that this datasource depends on the sqlite availability?

eguiraud · 2018-07-20T13:40:58Z

hi, alright i'm back at cern on monday, i'll review asap (and let you know about the roottest, but in first approximation you can follow what rarrowds does).

eguiraud · 2018-07-24T08:15:37Z

hi, cleaned up obsolete jenkins messages, will start a new build and start reviewing today

eguiraud · 2018-07-24T08:15:46Z

@phsft-bot build please

phsft-bot · 2018-07-24T08:24:00Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dimt=ON -Dccache=ON
How to customize builds

jblomer · 2018-08-16T05:43:04Z

It should be rebased now. Is there a standard ROOT method for printing warnings?

phsft-bot · 2018-08-16T05:49:42Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dccache=ON
How to customize builds

phsft-bot · 2018-08-16T05:51:30Z

Build failed on fedora28/native.
See console output.

Errors:

ninja: error: loading 'build.ninja': No such file or directory

phsft-bot · 2018-08-16T05:53:57Z

Build failed on windows10/vc15.
See console output.

Errors:

MSBUILD : error MSB1009: Project file does not exist.

phsft-bot · 2018-08-16T06:16:48Z

Build failed on centos7/gcc62.
See console output.

Failing tests:

projectroot.tmva.tmva.test.DNN.CNN.TMVA_DNN_CNN_Backpropagation_CPU

pcanal · 2018-08-16T14:28:35Z

Yes, TObject::Warning or ::Warning.

phsft-bot · 2018-08-16T15:18:08Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dccache=ON
How to customize builds

phsft-bot · 2018-08-16T15:32:21Z

Build failed on windows10/vc15.
See console output.

Errors:

MSBUILD : error MSB1009: Project file does not exist.

phsft-bot · 2018-08-16T16:00:41Z

Build failed on fedora28/native.
See console output.

Errors:

ninja: error: loading 'build.ninja': No such file or directory

eguiraud · 2018-08-27T13:34:50Z

@jblomer @dpiparo is this ready to go in?

jblomer · 2018-08-27T13:37:13Z

@bluehood Yes from my side

eguiraud · 2018-08-27T13:41:34Z

@phsft-bot build

phsft-bot · 2018-08-27T13:41:45Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dccache=ON
How to customize builds

phsft-bot · 2018-08-27T13:42:24Z

Build failed on fedora28/native.
See console output.

Errors:

ninja: error: loading 'build.ninja': No such file or directory

phsft-bot · 2018-08-27T13:45:54Z

Build failed on windows10/vc15.
See console output.

Errors:

MSBUILD : error MSB1009: Project file does not exist.

eguiraud · 2018-08-27T15:49:23Z

@jblomer @dpiparo both build failures are due to sqlite not being present on the machine.
@amadio suggests that the datasource be made optional in tree/dataframe/CMakeLists.txt, so it's not built when sqlite is not available.

jblomer · 2018-08-29T14:52:16Z

@bluehood @amadio @dpiparo The CMakeLists.txt file should already turn off this data source when sqlite is not available. Am I missing something?

amadio · 2018-08-29T15:00:16Z

tree/dataframe/CMakeLists.txt

@@ -22,11 +22,16 @@ if(NOT ARROW_FOUND)
  list(REMOVE_ITEM sources ${CMAKE_CURRENT_SOURCE_DIR}/src/RArrowDS.cxx)
 endif()

+if(NOT sqlite)


I think you may need to use ${ROOT_sqlite_FOUND} or equivalent here and elsewhere.

phsft-bot · 2018-08-29T22:47:38Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dccache=ON
How to customize builds

phsft-bot · 2018-08-29T22:48:20Z

Build failed on fedora28/native.
See console output.

Errors:

ninja: error: loading 'build.ninja': No such file or directory

phsft-bot · 2018-08-29T23:41:30Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dccache=ON
How to customize builds

phsft-bot · 2018-08-30T03:36:23Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dccache=ON
How to customize builds

eguiraud · 2018-08-30T08:51:47Z

thank you @jblomer for being so responsive to comments!

jblomer requested review from amadio, eguiraud and dpiparo as code owners July 16, 2018 07:28

amadio assigned eguiraud and dpiparo Jul 16, 2018

jblomer changed the title ~~WIP: SQlite data source for RDataFrame~~ SQlite data source for RDataFrame Jul 20, 2018

root-project deleted a comment from phsft-bot Jul 24, 2018

[DF] add warning for IMT with RSqliteDS

1699c5d

[DF] formatting fixes according to clang-format

7a84ebe

amadio reviewed Aug 29, 2018

View reviewed changes

jblomer added 2 commits August 30, 2018 00:01

[DF] fix includes for building RSqliteDS

5a7cc7c

[DF] More comments for sqlite ds

ac276b4

[DF] Don't link against SQlite libraries if not found

07a32c1

[DF] fix comment line

cbaf713

eguiraud merged commit 4c45a6d into root-project:master Aug 30, 2018

SQlite data source for RDataFrame #2322

SQlite data source for RDataFrame #2322

Uh oh!

Conversation

jblomer commented Jul 16, 2018

Uh oh!

jblomer commented Jul 20, 2018

Uh oh!

eguiraud commented Jul 20, 2018

Uh oh!

eguiraud commented Jul 24, 2018

Uh oh!

eguiraud commented Jul 24, 2018

Uh oh!

phsft-bot commented Jul 24, 2018

Uh oh!

jblomer commented Aug 16, 2018

Uh oh!

phsft-bot commented Aug 16, 2018

Uh oh!

phsft-bot commented Aug 16, 2018

Errors:

Uh oh!

phsft-bot commented Aug 16, 2018

Errors:

Uh oh!

phsft-bot commented Aug 16, 2018

Failing tests:

Uh oh!

pcanal commented Aug 16, 2018

Uh oh!

phsft-bot commented Aug 16, 2018

Uh oh!

phsft-bot commented Aug 16, 2018

Errors:

Uh oh!

phsft-bot commented Aug 16, 2018

Errors:

Uh oh!

eguiraud commented Aug 27, 2018

Uh oh!

jblomer commented Aug 27, 2018

Uh oh!

eguiraud commented Aug 27, 2018

Uh oh!

phsft-bot commented Aug 27, 2018

Uh oh!

phsft-bot commented Aug 27, 2018

Errors:

Uh oh!

phsft-bot commented Aug 27, 2018

Errors:

Uh oh!

eguiraud commented Aug 27, 2018

Uh oh!

jblomer commented Aug 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amadio Aug 29, 2018

Choose a reason for hiding this comment

Uh oh!

phsft-bot commented Aug 29, 2018

Uh oh!

phsft-bot commented Aug 29, 2018

Errors:

Uh oh!

phsft-bot commented Aug 29, 2018

Uh oh!

phsft-bot commented Aug 30, 2018

Uh oh!

eguiraud commented Aug 30, 2018

Uh oh!

Uh oh!

jblomer commented Aug 29, 2018 •

edited

Loading