Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Attempt to replace our IntoOnly with ExactlyOne #1915

Merged
merged 6 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions prql-compiler/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ use std::error::Error as StdError;
use std::fmt::{self, Debug, Display, Formatter};
use std::ops::{Add, Range};

use crate::utils::IntoOnly;

#[derive(Clone, PartialEq, Eq, Copy, Serialize, Deserialize)]
pub struct Span {
pub start: usize,
Expand Down Expand Up @@ -218,14 +216,6 @@ impl ErrorMessages {
}
}

impl IntoOnly for ErrorMessages {
type Item = ErrorMessage;

fn into_only(self) -> Result<Self::Item> {
self.inner.into_only()
}
}

impl ErrorMessage {
fn compose_display<'a, C>(&self, source_id: &'a str, cache: C, color: bool) -> Option<String>
where
Expand Down
1 change: 0 additions & 1 deletion prql-compiler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ mod test;
mod utils;

pub use error::{downcast, Error, ErrorMessage, ErrorMessages, Reason, SourceLocation, Span};
pub use utils::IntoOnly;

use once_cell::sync::Lazy;
use semver::Version;
Expand Down
4 changes: 2 additions & 2 deletions prql-compiler/src/semantic/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,15 +760,15 @@ fn env_of_closure(closure: Closure) -> (Module, Expr) {
mod test {
use anyhow::Result;
use insta::assert_yaml_snapshot;
use itertools::Itertools;

use crate::ast::pl::{Expr, Ty};
use crate::semantic::resolve_only;
use crate::utils::IntoOnly;

fn parse_and_resolve(query: &str) -> Result<Expr> {
let (stmts, _) = resolve_only(crate::parser::parse(query)?, None)?;

Ok(*stmts.into_only()?.kind.into_main()?)
Ok(*stmts.into_iter().exactly_one()?.kind.into_main()?)
}

fn resolve_type(query: &str) -> Result<Ty> {
Expand Down
6 changes: 4 additions & 2 deletions prql-compiler/src/sql/gen_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ use crate::ast::pl::{BinOp, JoinSide, Literal, RelationLiteral};
use crate::ast::rq::{CId, Expr, ExprKind, Query, RelationKind, TableRef, Transform};
use crate::sql::anchor::anchor_split;
use crate::sql::preprocess::SqlRelationKind;
use crate::utils::{BreakUp, IntoOnly, Pluck};
use crate::utils::{BreakUp, Pluck};

use crate::Target;

use super::context::AnchorContext;
Expand Down Expand Up @@ -249,7 +250,8 @@ fn sql_select_query_of_pipeline(

let projection = pipeline
.pluck(|t| t.into_super_and(|t| t.into_select()))
.into_only()
.into_iter()
.exactly_one()
.unwrap();
let projection = translate_wildcards(&ctx.anchor, projection);
let projection = translate_select_items(projection.0, projection.1, ctx)?;
Expand Down
2 changes: 1 addition & 1 deletion prql-compiler/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ mod only;
mod toposort;

pub use id_gen::{IdGenerator, NameGenerator};
use itertools::Itertools;
pub use only::*;
pub use toposort::toposort;

use anyhow::Result;
use itertools::Itertools;

pub trait OrMap<T> {
/// Merges two options into one using `f`.
Expand Down
59 changes: 5 additions & 54 deletions prql-compiler/src/utils/only.rs
Original file line number Diff line number Diff line change
@@ -1,46 +1,14 @@
use anyhow::{anyhow, Result};
use itertools::{Itertools, Position};
use anyhow::Result;

use crate::ast::pl::Expr;
use crate::error::{Error, Reason};

// Inspired by version in sqlparser-rs; I'm surprised there isn't a version in
// the stdlib / Itertools.
/// Returns the only element of an Iterator, or an error if it has more than one element.
pub trait IntoOnly {
type Item;

fn into_only(self) -> Result<Self::Item>;
}

impl<T, I> IntoOnly for I
where
I: IntoIterator<Item = T>,
// See below re using Debug.
// I: std::fmt::Debug,
// <I as IntoIterator>::IntoIter: std::fmt::Debug,
{
type Item = T;

fn into_only(self) -> Result<T> {
match self.into_iter().with_position().next() {
Some(Position::Only(item)) => Ok(item),
// Can't get the debug of the iterator because it's already
// consumed; is there a way around this? I guess we could show
// the items after the second, which is kinda weird.
Some(Position::First(_)) => Err(anyhow!("Expected only one element, but found more.",)),
None => Err(anyhow!("Expected one element, but found none.",)),
_ => unreachable!(),
}
}
}

pub trait IntoOnlyNode {
fn into_only_node(self, who: &str, occupation: &str) -> Result<Expr, Error>;
pub trait ExactlyOneNode {
fn exactly_one_node(self, who: &str, occupation: &str) -> Result<Expr, Error>;
}

impl IntoOnlyNode for Vec<Expr> {
fn into_only_node(mut self, who: &str, occupation: &str) -> Result<Expr, Error> {
impl ExactlyOneNode for Vec<Expr> {
fn exactly_one_node(mut self, who: &str, occupation: &str) -> Result<Expr, Error> {
match self.len() {
1 => Ok(self.remove(0)),
0 => Err(Error {
Expand All @@ -64,20 +32,3 @@ impl IntoOnlyNode for Vec<Expr> {
}
}
}

pub trait Only<T> {
fn only(&self) -> Result<&T>;
}

impl<T> Only<T> for [T]
where
T: std::fmt::Debug,
{
fn only(&self) -> Result<&T> {
if self.len() == 1 {
Ok(self.first().unwrap())
} else {
Err(anyhow!("Expected 1 item, got {}; {:?}", self.len(), self))
}
}
}
4 changes: 2 additions & 2 deletions prql-python/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![cfg(not(target_family = "wasm"))]
use std::str::FromStr;

use prql_compiler::{self, IntoOnly, Target};
use prql_compiler::{self, Target};
use pyo3::{exceptions, prelude::*};

#[pyfunction]
Expand All @@ -11,7 +11,7 @@ pub fn compile(prql_query: &str, options: Option<CompileOptions>) -> PyResult<St
.and_then(prql_compiler::pl_to_rq)
.and_then(|rq| prql_compiler::rq_to_sql(rq, &options.map(|o| o.into()).unwrap_or_default()))
.map_err(|e| e.composed("", prql_query, false))
.map_err(|e| (PyErr::new::<exceptions::PySyntaxError, _>(e.into_only().unwrap().reason)))
.map_err(|e| (PyErr::new::<exceptions::PySyntaxError, _>(e.to_string())))
max-sixty marked this conversation as resolved.
Show resolved Hide resolved
}

#[pyfunction]
Expand Down