You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "PragmaTwice (via GitHub)" <gi...@apache.org> on 2024/03/27 16:49:03 UTC
[PR] Add support for dumping DOT graphs from KQIR [kvrocks]
PragmaTwice opened a new pull request, #2205:
URL: https://github.com/apache/kvrocks/pull/2205
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for dumping DOT graphs from KQIR [kvrocks]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #2205:
URL: https://github.com/apache/kvrocks/pull/2205#issuecomment-2024931456
## [](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2205) **Quality Gate passed**
Issues
 [3 New issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=2205&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2205&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=2205&resolved=false&inNewCodePeriod=true)
 [61.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2205&metric=new_coverage&view=list)
 [2.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2205&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2205)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for dumping DOT graphs from KQIR [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on PR #2205:
URL: https://github.com/apache/kvrocks/pull/2205#issuecomment-2024807846
Ready for review now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for dumping DOT graphs from KQIR [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #2205:
URL: https://github.com/apache/kvrocks/pull/2205#discussion_r1543965597
##########
src/search/ir.h:
##########
@@ -22,33 +22,49 @@
#include <fmt/format.h>
+#include <initializer_list>
#include <limits>
#include <memory>
#include <optional>
#include <string>
+#include <type_traits>
#include <utility>
#include <variant>
#include <vector>
#include "fmt/core.h"
+#include "ir_iterator.h"
#include "string_util.h"
+#include "type_util.h"
// kqir stands for Kvorcks Query Intermediate Representation
namespace kqir {
struct Node {
virtual std::string Dump() const = 0;
+ virtual std::string_view Name() const = 0;
+ virtual std::string Content() const { return {}; }
Review Comment:
Not sure if my understanding is correct:
`Content` should be always overridden by the leaf node and `ChildBegin` and `ChildEnd` for the non-leaf node like SELECT and HASTAG query?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for dumping DOT graphs from KQIR [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #2205:
URL: https://github.com/apache/kvrocks/pull/2205#discussion_r1543971714
##########
src/search/ir.h:
##########
@@ -22,33 +22,49 @@
#include <fmt/format.h>
+#include <initializer_list>
#include <limits>
#include <memory>
#include <optional>
#include <string>
+#include <type_traits>
#include <utility>
#include <variant>
#include <vector>
#include "fmt/core.h"
+#include "ir_iterator.h"
#include "string_util.h"
+#include "type_util.h"
// kqir stands for Kvorcks Query Intermediate Representation
namespace kqir {
struct Node {
virtual std::string Dump() const = 0;
+ virtual std::string_view Name() const = 0;
+ virtual std::string Content() const { return {}; }
Review Comment:
No. `Content` is just to show some additional information about the node, e.g. in `NumericCompareExpr` will show the operator (==/</...).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for dumping DOT graphs from KQIR [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #2205:
URL: https://github.com/apache/kvrocks/pull/2205#discussion_r1543984450
##########
src/search/ir.h:
##########
@@ -22,33 +22,49 @@
#include <fmt/format.h>
+#include <initializer_list>
#include <limits>
#include <memory>
#include <optional>
#include <string>
+#include <type_traits>
#include <utility>
#include <variant>
#include <vector>
#include "fmt/core.h"
+#include "ir_iterator.h"
#include "string_util.h"
+#include "type_util.h"
// kqir stands for Kvorcks Query Intermediate Representation
namespace kqir {
struct Node {
virtual std::string Dump() const = 0;
+ virtual std::string_view Name() const = 0;
+ virtual std::string Content() const { return {}; }
Review Comment:
oh? I found some of non-leaf nodes(e.g. TagContainExpr/OrExpr) didn't implement the `Content`, so I thought the `Content` is only meaningful to the leaf node.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for dumping DOT graphs from KQIR [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #2205:
URL: https://github.com/apache/kvrocks/pull/2205#discussion_r1544012479
##########
src/search/ir.h:
##########
@@ -22,33 +22,49 @@
#include <fmt/format.h>
+#include <initializer_list>
#include <limits>
#include <memory>
#include <optional>
#include <string>
+#include <type_traits>
#include <utility>
#include <variant>
#include <vector>
#include "fmt/core.h"
+#include "ir_iterator.h"
#include "string_util.h"
+#include "type_util.h"
// kqir stands for Kvorcks Query Intermediate Representation
namespace kqir {
struct Node {
virtual std::string Dump() const = 0;
+ virtual std::string_view Name() const = 0;
+ virtual std::string Content() const { return {}; }
Review Comment:
Because there's no additional information in such node. Also, even for leaf node, if there is no additional information, its content can also be empty.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for dumping DOT graphs from KQIR [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #2205:
URL: https://github.com/apache/kvrocks/pull/2205#discussion_r1544027437
##########
src/search/ir.h:
##########
@@ -22,33 +22,49 @@
#include <fmt/format.h>
+#include <initializer_list>
#include <limits>
#include <memory>
#include <optional>
#include <string>
+#include <type_traits>
#include <utility>
#include <variant>
#include <vector>
#include "fmt/core.h"
+#include "ir_iterator.h"
#include "string_util.h"
+#include "type_util.h"
// kqir stands for Kvorcks Query Intermediate Representation
namespace kqir {
struct Node {
virtual std::string Dump() const = 0;
+ virtual std::string_view Name() const = 0;
+ virtual std::string Content() const { return {}; }
Review Comment:
i see, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for dumping DOT graphs from KQIR [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #2205:
URL: https://github.com/apache/kvrocks/pull/2205#discussion_r1544012479
##########
src/search/ir.h:
##########
@@ -22,33 +22,49 @@
#include <fmt/format.h>
+#include <initializer_list>
#include <limits>
#include <memory>
#include <optional>
#include <string>
+#include <type_traits>
#include <utility>
#include <variant>
#include <vector>
#include "fmt/core.h"
+#include "ir_iterator.h"
#include "string_util.h"
+#include "type_util.h"
// kqir stands for Kvorcks Query Intermediate Representation
namespace kqir {
struct Node {
virtual std::string Dump() const = 0;
+ virtual std::string_view Name() const = 0;
+ virtual std::string Content() const { return {}; }
Review Comment:
Because there's no additional information in such node. Also, even for leaf node, if there is no additional information, it's content can also be empty.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for dumping DOT graphs from KQIR [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice merged PR #2205:
URL: https://github.com/apache/kvrocks/pull/2205
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org